BrandonFors/pandas-github-issues huggingface dataset:
REF: make pytables get_atom_data non-stateful
State: closed

cc @WillAyd how do we annotate "this returns a subclass of Col"?  Here i've used `Type["Col"]` but afaik that doesnt include subclasses

Labels:
name: Refactor
description: Internal refactoring of code
name: IO HDF5
description: read_hdf, HDFStore

Comments:
I believed Type is covariant so should work for subclasses. Were you seeing an error?

Sent from my iPhone

> On Dec 4, 2019, at 5:27 PM, jbrockmendel <notifications@github.com> wrote:
> 
> ﻿
> cc @WillAyd how do we annotate "this returns a subclass of Col"? Here i've used Type["Col"] but afaik that doesnt include subclasses
> 
> You can view, comment on, or merge this pull request online at:
> 
>   https://github.com/pandas-dev/pandas/pull/30074
> 
> Commit Summary
> 
> REF: make pytables get_atom_data non-stateful
> File Changes
> 
> M pandas/io/pytables.py (29)
> Patch Links:
> 
> https://github.com/pandas-dev/pandas/pull/30074.patch
> https://github.com/pandas-dev/pandas/pull/30074.diff
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.

didnt see an error, was just curious
Yea Type is covariant by default so works for subclasses. TypeVar is invariant so worth knowing that distinction 

Sent from my iPhone

> On Dec 4, 2019, at 5:43 PM, jbrockmendel <notifications@github.com> wrote:
> 
> ﻿
> didnt see an error, was just curious
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.

updated to remove a comment explained the answer to.

also made unrelated changes to one comment typo and to re-raise on AssertionError
pls rebase
rebased+green
gentle ping, this is a blocker for the payoff ive been promising
#29886 - Replace !r for repr() on pandas/io/parses.py
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Refactor
description: Internal refactoring of code
name: Strings
description: String extension data type and string data

Comments:
Hello @JvPy! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2019-12-09 23:02:20 UTC
@jreback build fails if I remove repr from parsers.py. Can you help me with this issue?
thanks
STY: fstrings doc/source/conf.py
State: closed

- [x] ref #29547 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Code Style
description: Code style, linting, code_checks

Comments:
thanks
CI: troubleshoot codecov
State: closed

The results here https://codecov.io/gh/pandas-dev/pandas/tree/master/pandas are 17 days old, the last attempt to re-enable codecov apparently didn't take.  Trying again.

Labels:
name: CI
description: Continuous Integration

Comments:
Do we have any way (other than merging) to tell if this will actually accomplish the goal of getting codecov updated?
Any objections to the "merge this and see if it works" style of debugging here?
@WillAyd gentle ping
@thomasrockhu any ideas what we're doing wrong here?  pandas codecov page stopped updating about a month ago
@jbrockmendel taking a look now! thanks for bringing this to our attention
@jbrockmendel looks like it's a problem with how `$TYPE` is getting set (or that it's not getting set). As an example, right now the call to Codecov is

```
bash <(curl -s https://codecov.io/bash) -Z -c -F  -f /tmp/test_coverage.xml -C b4343efa33c9456694bcfd6dee7bf5941288966b
```

Since the flags `-F` flag is empty, it's pulling `-f` as the value. Let me know if this helps!
@thomasrockhu  Thanks for the quick feeedback.  It looks like the TYPE variable stopped getting defined in #28531, so im going to just remove that and see if it does the trick.
CLN: simplify pytables create_axes
State: closed

As mentioned in #30019, with Panel cases gone we can now deduce that `len(axes) == 1` in all cases in `create_axes`.  From there we can simplify this method a lot.  This PR does most of that; some of it is held back to avoid creating merge conflicts with #30067 (orthogonal)

Labels:
name: IO HDF5
description: read_hdf, HDFStore

Comments:
sure
STY: Style fix pandas/errors/__init__.py
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Code Style
description: Code style, linting, code_checks

Comments:
thanks
CI: correct azure-36-locale slow name + move pyarrow
State: closed

Follow-up on https://github.com/pandas-dev/pandas/pull/30039

I put it in the wrong file, because the file names were swapped...

Now, here, I just renamed both files to the other name (the diff apparently cannot the rename, therefore you see a lot of changes, but I can ensure I didn't change anything except for pyarrow). 
But, I could also leave the files as is and switch them in the posix.yml file ? (use the envs for the different build)

I don't know if there was something specific in either environment that was specifically added for the slow tests?

cc @datapythonista 

Labels:
name: CI
description: Continuous Integration

Comments:
is the test failure in the py37_locale build one of the things this PR is intended to address?
I didn't change anything about the 37 build (only 36), so no ;) 
I also didn't see that one yet, thanks for pointing out. It's from the PR I merged earlier today (https://github.com/pandas-dev/pandas/pull/29961, and which I should have updated with master after the CI changes to catch this failing test, in hindsight)
I quickly "fixed" CI (-> https://github.com/pandas-dev/pandas/pull/30069), will look into it tomorrow properly
> But if you know what you're doing, I'm happy with this

To be clear, I explicitly don't know what I am doing ;) Because there is no guideline or docs or rationale on what is the purpose of which build or what is supposed to be tested in which build. 

But, basically I didn't change anything in this PR (except for the pyarrow stuff), I just fixed the naming to be logical. The main thing I was wondering is that maybe the names weren't wrong in the first places, but we were accidentally using the wrong file in the posix.yml, and maybe I should only have changed it there (which would actually change which packages are used in the slow test build).
For the actual issue I was trying to solve: I checked that in the Linux py36_locale build pyarrow 0.12 is installed, and the parquet tests are actually run (not listed in the skipped ones), so now it is working as intended.
I was referring to unpinning all the packages. I think the renaming is the best we can do so far, at least the names match what's being tested. 
> I was referring to unpinning all the packages.

To be clear, in case you are talking about this PR: I am trying to explain I didn't do any of that. The "big diff" is just because git/github couldn't handle the rename
ah, I see, didn't get that. All good then. 
OK, merging then!
API: BooleanArray any/all with NA logic
State: closed

Closes https://github.com/pandas-dev/pandas/issues/29686

Implementation and tests for `any`/`all` with the updated logic as discussed in the linked issue.

Labels:
name: API Design

Comments:
> What happens with np.any with this? Do we need any keywords for compatibility?

Yes, still need to do that. If we want this to work (without getting into `__array_function__` for now), we need to add at least `axis` and `out`.

> Is the expected behavior here different from nanops.nanany? / nanops.nanall?

Ah, didn't look yet at those. They actually accept a `mask`. The approach they take is to fill the missing values with a `fill_value` (instead of filtering as I did here). 
But, we would still need the custom logic to decide when something should return pd.NA or not, so not fully sure it is worth to reuse those (will also do some timings tomorrow).

Also still need to add docstrings.
> Is the expected behavior here different from nanops.nanany? / nanops.nanall?

So I didn't use those methods, because indeed the behaviour that is now implemented in nanany/nanall for the `skipna=False` case is different.
This is good to go? 

The failure on Azure is the flaky resource warning thing.
Replace custom macro with stdlib 
State: closed

This is part of the C99 standard so I think better to leverage builtin functionality rather than redefine

Labels:
name: Clean

Comments:
LGTM pending green
test failure looks similar to #29976.
possibly related to #30065
You can ignore that error, did a quick "fix" to get CI green again for now: https://github.com/pandas-dev/pandas/pull/30069
Thanks @alimcmaster1 and @jbrockmendel for review
STY: fstrings doc/make.py
State: closed

- [x] ref #29547
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Code Style
description: Code style, linting, code_checks

Comments:
thanks @MomIsBestFriend
REF: pytables do string conversion early to set attributes in fewer places
State: closed

Next-step following #29979.

A lot of work is currently done in set_atom_string (a self-mutating method) that can be done in an earlier, not-mutating call.  By making that move, we simplify set_atom quite a bit, and get closer to getting all the attribute-setting into the constructor

Labels:
name: Refactor
description: Internal refactoring of code
name: IO HDF5
description: read_hdf, HDFStore

Comments:
thanks, that does simplify the nested logic a bit
i figured out last night it allows further simplfications even more than i expected.  coming up before long.
STY: "{foo!r}" -> "{repr(foo)}"
State: closed

- [x] ref #29886
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Code Style
description: Code style, linting, code_checks

Comments:
thanks @MomIsBestFriend
DOC: Style format
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
Can you fix up merge conflict?
@WillAyd I can't resolve this conflict, And now it's saying that I changed 150 files (no clue).

I think I will close this PR and open another one with the exact changes.
CI: fix mypy error
State: closed

`pandas/core/generic.py:9138: error: Item "None" of "Optional[DateOffset]" has no attribute "rule_code"`

https://github.com/pandas-dev/pandas/runs/333145832



Labels:
name: CI
description: Continuous Integration
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
thanks @TomAugspurger fixed it in this one: https://github.com/pandas-dev/pandas/pull/30049
actually i see you are fixing in a  better way, lmk what you want to do here
> actually i see you are fixing in a better way, lmk what you want to do here

Tom's fix is good for now.
CI: disable numpydev job
State: closed

xref https://github.com/pandas-dev/pandas/issues/30043.
Will take some time to fix all these cases.

Labels:
name: CI
description: Continuous Integration

Comments:
thanks @TomAugspurger 
merge on green.
grr
```
pandas/core/generic.py:9129: error: Item "None" of "Optional[DateOffset]" has no attribute "rule_code"
```
must be something i merged recently
Fixing the mypy issue in https://github.com/pandas-dev/pandas/pull/30049/commits/dd6b8171e549b498cda4c5d532d0e9f30cc2b2a5.
thanks @TomAugspurger
27820 clarify that read parquet accepts a directory path
State: closed

- [x] closes #27820 
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs
name: IO Parquet
description: parquet, feather

Comments:
Hello @HawkinsBA! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2019-12-05 07:53:07 UTC
I saw in the previous pull request for this issue that an example might be helpful. Is that still needed with these changes, and if so what would be the best way of going about that?
sure
CI: Fix print skipped tests
State: closed

- [X] closes #30040
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Looks like the format of the junit file that pytest generates has changed, and now tests are inside a `testsuite` tag, which makes the script to not find any tests. Instead of looking for them as immediate children of `testcase`, I look for them in all the tree (including subchildren).

CC: @jorisvandenbossche 

Labels:
name: CI
description: Continuous Integration

Comments:
looks good, thanks @datapythonista 
Thanks a lot for the quick fix!
CI: ensure proper coverage of different pyarrow versions
State: closed

Closes https://github.com/pandas-dev/pandas/issues/29223, closes https://github.com/pandas-dev/pandas/issues/26716

Labels:
name: CI
description: Continuous Integration
name: Dependencies
description: Required and optional dependencies

Comments:
Currently, pyarrow is included in the following builds:

- azure-macos-36 as `pyarrow>=0.12.0` -> this currently leads to version 0.13.0 in practice
- azure-windows-36 as `pyarrow>=0.12.0` -> this currently leads to version 0.13.0 in practice
- travis-37 as `pyarrow` -> this currently leads to 0.13.0 in practice.

The reason for this is that pyarrow is only at 0.13 on the default Anaconda channel. So for a more recent version, we will need one of our builds to use conda-forge.

We should also test the oldest supported version (0.12.0), this I added to `azure-36-minimum_versions`
OK, the build using conda-forge to have pyarrow 0.15 seems to work. 
I could also make this a new build instead of adapting an existing one (but to me that doesn't seem worth it)

The build with the minimum of 0.12 seems to be using arrow-cpp / pyarrow from conda-forge, because 0.12 is not available on the default channel (I think only 0.9 and 0.13). I am not sure this mixing of channels will work out, but it seems to go fine (although I can't check the tests actually run and are not skipped due to an import error, since the printing of skipped tests is not working anymore -> https://github.com/pandas-dev/pandas/issues/30040)
cc @jreback @datapythonista @alimcmaster1 
thanks @jorisvandenbossche
BUG: unclosed file warning when passing invalid encoding
State: closed

This isn't directly related to #30031 but might solve it if the problem there is unclosed files piling up.

TL;DR: the actual problem is in the stdlib `codecs.open`:

```
def open(filename, mode='r', encoding=None, errors='strict', buffering=1):
    if encoding is not None and 'b' not in mode:
        mode = mode + 'b'
    file = builtins.open(filename, mode, buffering)
    if encoding is None:
        return file
    info = lookup(encoding)
    srw = StreamReaderWriter(file, info.streamreader, info.streamwriter, errors)
    # Add attributes to simplify introspection
    srw.encoding = encoding
    return srw
```

When we pass a bogus encoding "foo" in test_format, the `lookup` call raises, and leaves behind the file opened a few lines earlier.

Labels:
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
Pretty interesting - is there something on bpo for this?
should we consider replacing all uses of codecs. used in a few other places?
seems the windows builds failed with this change for some reason.
> is there something on bpo for this?

Just opened https://bugs.python.org/issue38971

> should we consider replacing all uses of codecs. used in a few other places?

Maybe?  I'd want to track down why we're using codecs.open in the first place (best guess is a py2/py3 compat legacy)

> seems the windows builds failed with this change for some reason.

Yah, looks like we're getting '\n' instead of '\r\n' somewhere along the way.  Shouldn't be too hard to troubleshoot.


windows problem was fixed by passing `newline=""`
thanks
revert openpyxl test changes
State: closed

follow up to #30009 - thanks @jbrockmendel for fixing it and sorry I wasn't able to review more. I *think* the root cause though is just defusedxml being installed alongside things, which is only prevalent in our 3.6 build.

pinning openpyxl version alone I think is fine, but let's see...note that this same defusedxml issue causes a lot of warnings to be captured for xlrd in the excel tests

Labels:
name: CI
description: Continuous Integration

Comments:
Thanks for taking point on following up here; I'm troubleshooting the npdev build
Cool thanks. Rough day for CI
thanks @WillAyd
changed "fun !r" -> "repr(fun)"
State: closed

As described in the following issue
, usage of !r is currently redundant and so changing to f strings in place of it.

- [x] ref https://github.com/pandas-dev/pandas/issues/29886
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Code Style
description: Code style, linting, code_checks

Comments:
Hi, @WillAyd , are you saying to create another PR from my master branch (i am sorry, if not able to understand what you meant) or is there any other way to merge into master ?
> Hi, @WillAyd , are you saying to create another PR from my master branch (i am sorry, if not able to understand what you meant) or is there any other way to merge into master ?

no need to create another PR. see https://dev.pandas.io/docs/development/contributing.html#updating-your-pull-request
thanks @souravs17031999 CI failure is unrelated
REF: collect attribute-setting at the end of create_axes
State: closed

Ideally we wouldn't be altering `self` inplace at all, but we can make this a lot easier to reason about by collecting all of the attribute-setting at the end of the create_axes call.

`self.non_index_axes` I couldn't get all the way to the end of the call without a more invasive edit, which ill save for a separate pass to keep the diff smallish

Labels:
name: Refactor
description: Internal refactoring of code
name: IO HDF5
description: read_hdf, HDFStore

Comments:
changed to more descriptive names, added annotation for new_data_columns, non_index_axes i kept as `List`because I think we can do a better job of being consistent about what we're passing, want to wait to annotate until then 
sure
f strings in core/sorting
State: closed

- [ ] references #29547
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Code Style
description: Code style, linting, code_checks
name: Clean

Comments:
@donehower pls merge master
Thanks @donehower !
Updated .format() to f-strings
State: closed

- [ ] references #29547
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Code Style
description: Code style, linting, code_checks
name: Clean

Comments:
Thanks @dlalap congrats on first PR. If you'd like to address some suggestions from @jbrockmendel in a follow up would certainly welcome, along with any other contributions
fstring in io.sql
State: closed

- [ ] ref #29547
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Code Style
description: Code style, linting, code_checks
name: Clean

Comments:
@lexy-lixinyu can you commit changes as suggested by @simonjayhawkins ?
just pushed edits removing unintentional " " in two places.
Thanks for the work on this @lexy-lixinyu, can you address the two last comments, and update your branch from master please? This is ready to be merged after it.

If you need help let us know.
just pushed edits to address outstanding comments
Thanks @lexy-lixinyu and @jbrockmendel
f strings in core/series
State: closed

- [ ] references #29547
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Code Style
description: Code style, linting, code_checks
name: Clean

Comments:
pls merge master as well
Thanks @donehower keep them coming
Type Annotations for df.boxplot
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
Thanks for the PR: Could you please sort your imports using isort
```
4.3.21
Check import format using isort
ERROR: /home/runner/work/pandas/pandas/pandas/plotting/_core.py Imports are incorrectly sorted.
```
closing as stale, if you'd like to continue, pls ping.
f-string update for core.base.
State: closed

- [ ] ref #29547
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Code Style
description: Code style, linting, code_checks
name: Clean

Comments:
Done. All went fine. Do I need to do another pull request or is this all
set.

Darin Plutchok
206-909-3665


On Tue, Dec 3, 2019 at 8:48 PM William Ayd <notifications@github.com> wrote:

> *@WillAyd* requested changes on this pull request.
>
> Thanks for the PR. Looks good and I think CI failures are unrelated / have
> been fixed on master since this was created.
>
> To be sure, can you locally on your feature-firstcontrib branch run:
>
> git fetch upstream
> git merge upstream/master
> git push origin feature-firstcontrib
>
> I think should get CI green for this. Flag me down at PyData if you have
> any questions
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/30023?email_source=notifications&email_token=AAYK476CPUWZEZ2ZTSU26CTQW4ZDBA5CNFSM4JVAK22KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCN3YPHQ#pullrequestreview-326600606>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAYK475B5N7KECJFTSGPOXTQW4ZDBANCNFSM4JVAK22A>
> .
>

Don't need a new PR; just keep pushing to this. Looks like CI is running so let's see
thanks @dplutcho