BrandonFors/pandas-github-issues huggingface dataset:
REGR: SeriesGroupBy where index has a tuple name fails
State: closed

- [x] closes #37755
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

The groupby code that calls `Series.__getitem__` catches `KeyError` and `TypeError`. If a tuple is fed into an `Index` that is not a `MultiIndex`, we were raising a `ValueError` but `KeyError` seems more appropriate here and also fixes the regression.

While working on this, I found that we raise a `KeyError` inappropriately if an index of tuples contains duplicates, however this exists back in 1.0.x and so is not part of the regression. I've opened #37800 for this.

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Regression
description: Functionality that used to work in a prior pandas version
name: Series
description: Series data structure

Comments:
thanks @rhshadrach keem em coming!
@meeseeksdev backport 1.1.x
Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

```
$ git checkout 1.1.x
$ git pull
```

2. Cherry pick the first parent branch of the this PR on top of the older branch:
```
$ git cherry-pick -m1 be03eb13fdb97d1231ec99f162f9bc0c170d50e1
```

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

```
$ git commit -am 'Backport PR #37801: REGR: SeriesGroupBy where index has a tuple name fails'
```

4. Push to a named branch :

```
git push YOURFORK 1.1.x:auto-backport-of-pr-37801-on-1.1.x
```

5. Create a PR against branch 1.1.x, I would have named this PR:

> "Backport PR #37801 on branch 1.1.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to [suggest an improvement](https://github.com/MeeseeksBox/MeeseeksDev).
BUG: DataFrame.all(bool_only=True) inconsistency with object dtype
State: closed

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

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: API - Consistency
description: Internal Consistency of API/Behavior

Comments:
ping
BUG: Bug in DataFrame.loc for setitem changed dtype when setting more than one column
State: closed

- [x] closes #18415
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Could not find a reason why we should convert the dtype to object, if values is already an array.

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Stale

Comments:
Hello @phofl! 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 2020-12-23 23:51:05 UTC
Done
cc @jreback  Merged master
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
@phofl can you merge master to resolve conflicts
@phofl status on this one?
@phofl closing as stale. reopen when ready
TYP: fix mypy ignored error in pandas/tests/io/test_fsspec.py
State: closed

- [ ] xref #37715
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Handle mypy ignored error in pandas/tests/io/test_fsspec.py

Is this decision correct?

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

Comments:
thanks @Mikhaylov-yv 
hey @Mikhaylov-yv , can I work on it?
@nitin-pandita 
I think yes.  What did you want to do here?
BUG: Slicing non monotonic DatetimeIndex did not raise for non-existing keys
State: closed

- [x] closes #18531
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

The test for historical reasons :) failed after the change. Is this a problem?


Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Error Reporting
description: Incorrect or improved errors from pandas

Comments:
If there are "historical reasons", then we could also deprecate it first.
Also, we should clean up the comments about this (as they are now no longer correct)

@phofl can you also add a test where the resulting dataframe would not be empty? So where eg the start label is missing but would fall somewhere in the middle of the range of the index labels
> If there are "historical reasons", then we could also deprecate it first.
> Also, we should clean up the comments about this (as they are now no longer correct)
> 
> @phofl can you also add a test where the resulting dataframe would not be empty? So where eg the start label is missing but would fall somewhere in the middle of the range of the index labels

i am not sure there is anything to deprecate, this is just a bug
It's also explicitly tested behaviour (I don't know the motivations of the PR that added this test, though)
> It's also explicitly tested behaviour (I don't know the motivations of the PR that added this test, though)

its wrong it should raise, pls see the OP
Sorry for the confusion with the other issue. It raises for non-monotonic indexes and not for monotonic indexes
> its wrong it should raise, pls see the OP

I agree the behaviour seems wrong and we should change it. To be clear I am not arguing for not having it raise an error. I am only saying that if this is explicitly tested behaviour, it *might* be depended upon (again, I don't know when / for what reason this test was added), and we can consider deprecating it first.
> > its wrong it should raise, pls see the OP
> 
> I agree the behaviour seems wrong and we should change it. To be clear I am not arguing for not having it raise an error. I am only saying that if this is explicitly tested behaviour, it _might_ be depended upon (again, I don't know when / for what reason this test was added), and we can consider deprecating it first.

fair. ok let's see if possible to deprecate. though am tempted to call this wrong and just a bug fix but let's see.
Opened #37819 to deprecate this. If we decide to deprecate first, we can close this one.
closing in favor of #37819
[BUG]: Fix bug in MultiIndex.drop dropped nan when non existing key was given
State: closed

- [x] closes #18853
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Additional question:
The doc says (https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.Index.drop.html) that ``drop`` raises when not all labels are found, but
```
pd.DataFrame(index=pd.MultiIndex.from_product([range(3), range(3)])).drop([1, 5], level=0)
```
does not raise, because ``1`` is found. Should we implement, that it raises every time when at least one of the keys is not found?

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: MultiIndex

Comments:
Hello @phofl! 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 2020-11-13 13:20:13 UTC
cc @jreback failure unrelated. Greenish
thanks @phofl
Remove unnecessary casts in tokenizer.c
State: closed

Some of these are visual clutter, some are actual bugs though in unlikely cases (see the format specifiers)

https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc


Labels:
name: IO CSV
description: read_csv, to_csv
name: Clean

Comments:
There are no warnings now because the casts would suppress them, but this fixes a bug when the number of rows in a CSV file exceeds the size of a long long 
thanks
BUG: freq inference for tz-aware DatetimeIndex
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: Mothballed
description: Temporarily-closed PR the author plans to return to

Comments:
mothballing while troubleshooting locally
Fix regression for loc and __setitem__ when one-dimensional tuple was given for MultiIndex
State: closed

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


Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Regression
description: Functionality that used to work in a prior pandas version
name: MultiIndex

Comments:
@phofl you're opening PRs faster than they can be reviewed (which is awesome, keep up the good work!).  Maybe you can tackle one of the more challenging and immediate issues: the darns windows builds
Thank you very much @jbrockmendel Have a bit of free time this week.

Are you referring to the pipeline windows py38_no18? Unfortunately I am not that familiar with CI and even less with CI on windows.
lgtm ex @jbrockmendel comment, ping on green.
thanks @phofl keep em coming!
@meeseeksdev backport 1.1.x
>> you're opening PRs faster than they can be reviewed (which is awesome, keep up the good work!). Maybe you can tackle one of the more challenging and immediate issues: the darns windows builds

> Are you referring to the pipeline windows py38_no18? Unfortunately I am not that familiar with CI and even less with CI on windows.

CI is just another learning curve we all have to get up at some point. i still regularly ask for help with it.

As for windows, im not actually convinced anyone uses it ever.  It might be some kind of cruel prank played by a trickster god.  see #37746 where i only-half-kidding suggest we drop windows altogether.

More generally, I'm suggesting you look for areas that will have a higher ratio of (time to create PR) / (time to review PR).\*

\* Though im really excited about the indexing fixes you've been making.
CI/TST: use https to avoid ResourceWarning in html tests
State: closed

See #36467 
(no idea if this would actually fix anything, just mimicking what @alimcmaster1 did in https://github.com/pandas-dev/pandas/pull/36480)

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: IO HTML
description: read_html, to_html, Styler.apply, Styler.applymap

Comments:
At least it passed once .. ;)
Also passed a second time, so let's merge to see what happens
@simonjayhawkins is this something that needs to be backported?
> @simonjayhawkins is this something that needs to be backported?

the coverage build is failing on 1.1.x, #37309, so no need to backport for now.
REF: simplify Block.replace
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: Refactor
description: Internal refactoring of code
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
I guess this technically a bug fix as previously replace on a Period would incorrectly cast, pls add a whatsnew, merge on green (or ping if you want).
BUG: adding Timedelta to DatetimeIndex raising incorrectly
State: closed

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


Labels:
name: Datetime
description: Datetime data dtype
name: Regression
description: Functionality that used to work in a prior pandas version
name: Timedelta
description: Timedelta data type

Comments:
very nice
BUG: Groupby head/tail with axis=1 fails
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

This potentially closes #9772. The other issue there involves the perhaps unexpected fact that using `axis=1` with `groupby(...).apply` does _not_ transpose the groups that the function is going to be applied to. I've expressed my thoughts on why we should not do so in https://github.com/pandas-dev/pandas/issues/9772#issuecomment-723498819.

Labels:
name: Bug
name: Groupby

Comments:
thanks @rhshadrach very nice.

agree with your rationale for the other part of #9772. I guess its worth opening an issue about it though just to track.
BUG: Bug in xs ignored droplevel
State: closed

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


Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: MultiIndex

Comments:
thanks @phofl
Bug in __getitem__ incorrectly dropped columns for empty Series as indexer
State: closed

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


Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves

Comments:
It isn't obvious to me that this is wrong in master.  In the test, `ind` is object dtype, which should be treated as an indexer along columns, so `df[ind]` should match `df[[]]`, which it does in master.  If instead we do `df[ind.astype(bool)]` its indexing along rows and the columns are retained
I interpreted the issue the way, that both should do the same. If this is not the case, we can close this PR and the associated issue.
can you merge master
Merged master, but no longer sure if this fix is doing the correct think as @jbrockmendel  mentioned
@jreback i think this is right on master https://github.com/pandas-dev/pandas/pull/37772#issuecomment-725715870
> @jreback i think this is right on master [#37772 (comment)](https://github.com/pandas-dev/pandas/pull/37772#issuecomment-725715870)

can you clarify? 
> > @jreback i think this is right on master https://github.com/pandas-dev/pandas/pull/37772#issuecomment-725715870

> can you clarify?

```
df = DataFrame(columns=["a", "b", "c"])

v1 = Series([], dtype=object)
v2 = Series([], dtype=bool)

df[v1]  # <-- object dtype, so indexing on columns, so we expect to get `DataFrame()`
df[v2]  # <-- bool dtype, so indexing on rows, so we expect to get `df`
```

This PR (and the associated issue) are changing `df[v1]` to match `df[v2]`, which it shouldnt.
> > > @jreback i think this is right on master [#37772 (comment)](https://github.com/pandas-dev/pandas/pull/37772#issuecomment-725715870)
> 
> > can you clarify?
> 
> ```
> df = DataFrame(columns=["a", "b", "c"])
> 
> v1 = Series([], dtype=object)
> v2 = Series([], dtype=bool)
> 
> df[v1]  # <-- object dtype, so indexing on columns, so we expect to get `DataFrame()`
> df[v2]  # <-- bool dtype, so indexing on rows, so we expect to get `df`
> ```
> 
> This PR (and the associated issue) are changing `df[v1]` to match `df[v2]`, which it shouldnt.

OH! ok
Closing this then? And the associated issue if this is the expected behavior?
> Closing this then? And the associated issue if this is the expected behavior?

yes
TST: Isin converted floats unnecessarily to int causing rounding issues
State: closed

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


Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: isin
description: isin method

Comments:
The problem I was facing is, that we sometimes get numpy arrays from dtype objects actually containing integers/floats, which we would like to cast to integer to try isin. So  we can not really filter for integer dtypes on ``values``. But sometimes they contain strings, which leads to raise in ``maybe_upcast``, hence the try except block. I am not patricularly happy with this solution either, but I did not know a better way to convert to integer dtype if possible and pass in all other cases.
I think a recent PR from @jbrockmendel may have obviated the need for this, but merge master and let's see
No, ``ensure_data`` is still called without taking the dtype of ``values`` into account. Causes the same problems as before unfortunately
ive been looking at `_ensure_data` from another angle, and it basically needs to be split into two functions.  one for everything besides `isin`, in which `dtype` is never passed, and another for `isin` in which we need to pass both `comps` and `values` at the same time and use something like `find_common_dtype`
That would work, yes. Will look into this during the weekend
The one usage of _ensure_data that passed `dtype` has now been removed, so that can likely be simplified now.
Your commit yesterday fixed above cases, so only adding tests here.
this might be fixed on master now, can youi revisit
Yes was fixed by @jbrockmendel, hence only adding tests here
excellent, thanks @phofl
Bug in loc did not raise KeyError when missing combination with slice(None) was given
State: closed

- [x] closes #19556
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Additional question: When an non existent key is given for ``get_locs`` it raises a ``KeyError``. When just a missing combination of keys is given (``(b,1)`` in the new test), while the individual keys exist, an empty array is returned. Is this the expected behavior?

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: MultiIndex

Comments:
...

Edit: Did not think this through. 
lgtm @jbrockmendel if any comments.
LGTM

Thanks for tackling all these indexing issues @phofl, they can be a real doozy.  FWIW we've done a lot of semi-recent cleanup of the indexing code but MultiIndex has received relatively little attention.  Probably some relatively low-hanging fruit there.
Thanks, 

I will have a look for the MultiIndex code paths
@phofl can you merge master, the CI should be fixed
Merged master
thanks @phofl
BUG: clean the figure windows created by tests
State: closed

- [x] closes #35080
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

fixed: FAILED pandas/tests/plotting/test_datetimelike.py::TestTSPlot::test_ts_plot_with_tz['UTC']
reference:
- https://github.com/pandas-dev/pandas/issues/35080#issuecomment-652988132 @StefRe as co-author (have already marked as  the co-author in my forked repo, however, I'm not sure whether as well as right here)
- https://stackoverflow.com/a/8228808/7069618
- https://stackoverflow.com/a/33343289/7069618

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Unreliable Test
description: Unit tests that occasionally fail

Comments:
thanks @onshek
DOC: test organization
State: closed

Retry of #37637, reverted by #37756

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Docs

Comments:
@jorisvandenbossche thoughts on the rst?
@jorisvandenbossche thanks for the notes, implementing them now
thanks @jbrockmendel 

merging this, will likely be updating as we go so if more comments can handle then.
TYP: Extend typing to allow passing dates to to_datetime
State: closed

Updates the typing to reflect what is already possible.

Otherwise, Pycharm is pretty unhappy when a date is pased.

- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`


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

Comments:
needs to merge master and update
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
closing as stale. if you want to continue, pls ping and can re-open.
DOC: troubleshoot docbuild
State: closed


Labels:

Comments:
@jbrockmendel I reverted the original PR (https://github.com/pandas-dev/pandas/pull/37756), so that 1) other PRs don't miss doc errors because of CI being red and also 2) it's easier to give inline feedback on an open PR.

So if you reopen the original PR (or open a new one), then I am happy to give some feedback on the rst formatting.
perfect, thank you