BrandonFors/pandas-github-issues huggingface dataset:
ENH: support Ellipsis in loc/iloc
State: closed

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

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

Comments:
I realize there is still an open issue for this but it is quite old. What do you see as the main motivation to support this given we no longer have a Panel?
> What do you see as the main motivation to support this given we no longer have a Panel?

As @shoyer mentioned in #10956, this makes it easier to write code that is valid for both Series and DataFrame, e.g. `obj.loc[..., labels]` 

In particular Im finding this would be useful for parametrizing indexing tests.
I agree this is good for general indexing compatiblity and is unambiguous.

@jbrockmendel can you make this raise for a MI or is it unambiguous there? (I think it is more useful than the slice syntax, but that should be a dedicated PR).

cc @jorisvandenbossche 
> can you make this raise for a MI or is it unambiguous there? (I think it is more useful than the slice syntax, but that should be a dedicated PR).

MI cases are appreciably more complicated and im planning to handle them separately
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.
i think this is ok, we should be able to accept ellipsis (even if not that useful). add a whatsnew and rebase. ping on green (i think @jorisvandenbossche had a comment as well).
@jbrockmendel can you rebase this
> can you rebase this

sure, but there are still a handful of comments I havent addressed, so this isnt ready
updated to flesh out tests, i think comments are addressed
@jbrockmendel if this is ready can you add a release note and a section on the new capability.
whatsnew added + green
@jbrockmendel needs rebase
rebased + green
> Actually i dont' think this is any different than : right ultimately right?

The main thing this allows is to write code that works for either Series or DataFrame `obj.loc[..., "foo"]`.  IIRC the original motivation was to make sharing tests easier.
> > Actually i dont' think this is any different than : right ultimately right?
> 
> The main thing this allows is to write code that works for either Series or DataFrame `obj.loc[..., "foo"]`. IIRC the original motivation was to make sharing tests easier.

ahh got it. then am +1 here.

cc @phofl if any comments.
can you create an issue for adding docs about this in the user guide / doc-string as appropriate.
> `df = pd.DataFrame({"a": [1, 2, 3]}).loc[(..., ), :]` valid syntax?

If you mean "should it be equivalent to `.loc[..., :]`?" then no.  But...

```
index = [0, Ellipsis, NotImplemented]
df = pd.DataFrame({"a": [1, 2, 3]}, index=index)

>>> df.loc[(..., ), :]
          a
Ellipsis  2
```

(on master, not PR branch)
thanks @jbrockmendel
CLN: remove unused min_count argument in libgroupby.group_nth
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: Groupby
name: Clean

Comments:
Does this have any impact to an end user?
> Does this have any impact to an end user?

Good point. I *think* it allows the user to pass `min_count` to `group_nth`. Will see if I can come up with a test
thanks @arw2019
CI: troubleshoot windows builds
State: closed

Windows builds are still hitting what look like OOM errors.  Trying splitting these into smaller pieces; its fragile.

Labels:
name: CI
description: Continuous Integration
name: Windows
description: Windows OS

Comments:
Any idea which set of tests are likely to cause memory issues? Maybe matplotlib tests?
> Any idea which set of tests are likely to cause memory issues? Maybe matplotlib tests?

hard to tell; in most cases there isnt a traceback pointing to a test
new idea: lets just drop windows altogether
@datapythonista help?  ive been banging my head against this windows build and am getting nowhere, somehow managed to break a linux build for a little while there.

Could we disable xdist and see if we can get more meaningful exception messages?

Could we split this into multiple builds to avoid the time limit?
@jbrockmendel how about one more rebase here and then let's merge and call it a day.
> how about one more rebase here and then let's merge and call it a day.

just rebased, but i got a _lot_ of failing builds before getting one to pass.  lets try to get at least a few in a row before merging (and make sure we are OK with dropping all these optional dependencies)
HOW DID THIS BREAK THE LINUX BUILDS
> HOW DID THIS BREAK THE LINUX BUILDS

broken elsewhere, pending patch for that.
> broken elsewhere, pending patch for that.

is that related to the something.xls and something.xlsx files that im seeing left behind after running the tests for the last day or two?
> is that related to the something.xls and something.xlsx files that im seeing left behind after running the tests for the last day or

damn, I was wondering where those came from. `ExcelWriter` now opens the files which changed the behavior of this test `pandas/tests/io/excel/test_writers.py:test_register_writer`. This test wasn't expecting that an empty file will be created so it also didn't clean them up.

I will make a PR.

edit: #37857
Is `xvfb-run: error: Xvfb failed to start` showing in the linux builds for any other PR?  bc if not, then this is somehow breaking those builds.
Sorry for the delay @jbrockmendel. I've been having a look, but I'm missing some context.

The current failure seems to be a timeout. There is something I don't quite understand on it, seems like after finishing the tests, which take around 30 minutes, another call to pytest starts, and it doesn't finish on time. But this seems unrelated to the out of memory problems you mention.

Does this make sense? Is the problem I'm seeing the one you're having trying to address?
trying nth time, if green let's just merge 
wow 2 in a row, merging.
@jbrockmendel i thought we had an issue about troubleshooting these builds, can you update it to say now windows/plotting excluded.
TYP: follow-up to #37723
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:
see https://mypy.readthedocs.io/en/stable/generics.html#declaring-decorators

we have `F` defined in pandas._typing for this purpose.
> we have F defined in pandas._typing for this purpose.

```
pandas/core/nanops.py:400: error: Incompatible return value type (got "Callable[..., Any]", expected "F")  [return-value]
```

The functions being decorated dont all have the same signature, so the decorated func has `**kwargs` that the targets dont have
need to use `return cast(F, new_func)`
TYP: fix mypy ignored err in pandas/io/formats/format.py
State: closed

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

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

Comments:
@simonjayhawkins it's green
> @simonjayhawkins it's green

since this is changing code, will wait for 2nd approval.



LGTM
Thanks @ivanovmg
TYP: fix mypy ignored error in pandas/io/formats/latex.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/io/formats/latex.py``.

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

Comments:
@simonjayhawkins looks like green (internal Windows py38_np18 error)
Thanks @ivanovmg
BUG: read_html - file path cannot be pathlib.Path type
State: closed

Fix `read_html` TypeError when parsing `pathlib.Path` object.


Labels:
name: IO HTML
description: read_html, to_html, Styler.apply, Styler.applymap

Comments:
Thanks for the reply! @arw2019 

> Can you add tests? 

Do you mean adding test cases in the test_html.py?

> You'll also need a whatsnew entry (probably 1.2) 

Can you explain a little bit more what is the whatsnew entry and where should I start? 

This is actually my first open-source contribution and I'm still trying to get familiar with everything. :)
> Thanks for the reply! @arw2019
> 
> > Can you add tests?
> 
> Do you mean adding test cases in the test_html.py?
 
Yes that sounds like the place

> > You'll also need a whatsnew entry (probably 1.2)
> 
> Can you explain a little bit more what is the whatsnew entry and where should I start?

We document user-facing changes to the library on the whatsnew docs page. Here is the most recent one: 
https://github.com/pandas-dev/pandas/blob/master/doc/source/whatsnew/v1.2.0.rst

The entry for this patch should probably go under `Bug fixes`, `I/O`. Take a look at other entries, have a go at writing yours and we'll give comments

> This is actually my first open-source contribution and I'm still trying to get familiar with everything. :)

Welcome to pandas!
Thanks for the comments! I will follow up with the test case and whatsnew entry. @arw2019 
Hello @inspurwusixuan! 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-10 23:34:15 UTC
thanks @inspurwusixuan
TST: parametrize in tests/plotting/test_frame.py
State: closed

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

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

Comments:
/azp run
<samp>
Azure Pipelines successfully started running 1 pipeline(s).<br>

</samp>
very nice!
@ivanovmg if you wouldn't mind can you mark most of these parameterized tests as slow. we are timing out are builds pretty offten: https://travis-ci.org/github/pandas-dev/pandas/jobs/745534073

and its taking like 45min. i don't think in and of themselves these tests are slow, but they do take some time.  the slow builds have more room these tests.
> 
> 
> @ivanovmg if you wouldn't mind can you mark most of these parameterized tests as slow. we are timing out are builds pretty offten: https://travis-ci.org/github/pandas-dev/pandas/jobs/745534073
> 
> and its taking like 45min. i don't think in and of themselves these tests are slow, but they do take some time. the slow builds have more room these tests.

Sure, I will go through it and mark the slowish tests as slow.
> > @ivanovmg if you wouldn't mind can you mark most of these parameterized tests as slow. we are timing out are builds pretty offten: https://travis-ci.org/github/pandas-dev/pandas/jobs/745534073
> > and its taking like 45min. i don't think in and of themselves these tests are slow, but they do take some time. the slow builds have more room these tests.
> 
> Sure, I will go through it and mark the slowish tests as slow.

great. likley let's mark most / all of them. the fast builds are just taking too long and timing out.
TST: use default_axes=True where multiple plots
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Use new kwarg ``default_axes`` in ``_check_plot_works`` (#37274), which allows one to eliminate catching warnings related to multiple subplots created.
There warnings are not related to the plotting function, but rather were caused by the ``_check_plot_works`` itself.


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Visualization
description: plotting
name: Clean

Comments:
Thanks @ivanovmg
API: consistently raise TypeError for invalid-typed fill_value
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

This makes us consistent about raising TypeError when passing a wrong-typed fill_value to e.g. `take`.  The downside is that we changed `Categorical.take` from TypeError to ValueError back in 1.1.0 #33660, so theres a bit of whiplash.

The upside (besides this just being More Correct) is that we will be just about fully sharing setitem-like validator methods.

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:
Yea the back and forth is unfortunate, but I agree this seems more correct. Could use a whatsnew note
thanks
CLN: clean setup.py
State: closed

an attempt to clean setup.py

Labels:
name: Build
description: Library building on various platforms

Comments:
We don’t need to go into the details - just want to make sure this doesn’t cause regressions like the build time slowdown called out before

Sent from my iPhone

> On Nov 11, 2020, at 2:21 PM, Fangchen Li <notifications@github.com> wrote:
> 
> ﻿
> @fangchenli commented on this pull request.
> 
> In setup.py:
> 
> >  if _CYTHON_INSTALLED:
> -    from Cython.Distutils.old_build_ext import old_build_ext as _build_ext
> -
> -    cython = True
> -    from Cython import Tempita as tempita
> +    from Cython import Tempita
> +    from Cython.Distutils.build_ext import build_ext as _build_ext
> Looking at the cython source code. The new build_ext is just
> setuptools.command.build_ext or distutils.command.build_ext. But I'm not sure how it differs from the old_build_ext.
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or unsubscribe.

> We don’t need to go into the details - just want to make sure this doesn’t cause regressions like the build time slowdown called out before
> […](#)
> Sent from my iPhone
> On Nov 11, 2020, at 2:21 PM, Fangchen Li ***@***.***> wrote: ﻿ @fangchenli commented on this pull request. In setup.py: > if _CYTHON_INSTALLED: - from Cython.Distutils.old_build_ext import old_build_ext as _build_ext - - cython = True - from Cython import Tempita as tempita + from Cython import Tempita + from Cython.Distutils.build_ext import build_ext as _build_ext Looking at the cython source code. The new build_ext is just setuptools.command.build_ext or distutils.command.build_ext. But I'm not sure how it differs from the old_build_ext. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

Sure, the windows np16 build does seem slower than before. I'll test it more.
The runtime for the windows build seems normal now.
thanks @fangchenli
REF: use extract_array in DataFrame.combine_first
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

Comments:
lgtm cc @jbrockmendel 
the extract_array part looks good, but not clear why the needs_i8_conversion part can be removed safely
> the extract_array part looks good, but not clear why the needs_i8_conversion part can be removed safely

I reverted it

Although CI is not passing without for lack of testing with datelike input - AFAICT we have good (but maybe not water-tight) coverage there
> Although CI is not passing without for lack of testing with datelike input - AFAICT we have good (but maybe not water-tight) coverage there

it may be unnecessary (in fact i hope it is), just want to have an idea why its unnecessary (or at least make sure we have relevant test coverage)
> > Although CI is not passing without for lack of testing with datelike input - AFAICT we have good (but maybe not water-tight) coverage there
> 
> it may be unnecessary (in fact i hope it is), just want to have an idea why its unnecessary (or at least make sure we have relevant test coverage)

Ok! Checked that both parts of the if/else clause inside `if needs_i8_conversion(arr.dtype)` are hit in:
```
pandas/tests/frame/methods/test_combine_first.py::TestDataFrameCombineFirst::test_combine_first_timezone
pandas/tests/frame/methods/test_combine_first.py::TestDataFrameCombineFirst::test_combine_first_period
```
thanks @arw2019
Bug in iloc aligned objects
State: closed

- [x] closes #22046
- [x] closes #37593
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

If iloc should be purely position based, this PR fixes the bug described in the issue. One test depends on the wrong behavior, so I had to adjust it.

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

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-18 20:26:46 UTC
thanks @phofl nice!
agreed thanks @phofl, this wasnt easy, glad to see it fixed
Thanks @jbrockmendel  and @jreback 
Will add a followup to adress comments.
ENH: Raise when subsetting columns on groupby with axis=1
State: closed

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


Labels:
name: Enhancement
name: Groupby
name: Error Reporting
description: Incorrect or improved errors from pandas

Comments:
/azp run
<samp>
Azure Pipelines successfully started running 1 pipeline(s).<br>

</samp>
thanks @rhshadrach
TST: Add test for filling new rows through reindexing MultiIndex
State: closed

- [x] closes #23693
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Issue was fixed previously

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: MultiIndex
name: Needs Tests
description: Unit test(s) needed to prevent regressions

Comments:
thanks @phofl
REF: remove Categorical._validate_listlike, unbox_listlike
State: closed

Getting closer to having these standardized.

Labels:
name: Refactor
description: Internal refactoring of code
name: Categorical
description: Categorical Data Type

Comments:
assuming this is perf neutral.
> assuming this is perf neutral.

very slight improvement in reshape.merge from doing `is_foo_dtype(x.dtype)` instead of `is_foo_dtype(x)`, other than that totally neutral.
BUG: loc.__getitem__[[na_value]] with CategoricalIndex containing NAs
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: Bug
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Categorical
description: Categorical Data Type

Comments:
ok i guess this is worth a whatsnew note, look forward to see how you explain what is changing.
Whatsnew added + green
TST: Add test for missing fillvalue in categorical dtype
State: closed

- [x] closes #18185
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

cc @jreback Could not find a test for something similar with reindex.

Labels:
name: Categorical
description: Categorical Data Type
name: Needs Tests
description: Unit test(s) needed to prevent regressions

Comments:
thanks!
TST: Add test for KeyError with MultiIndex
State: closed

- [x] closes #25236
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

cc @jreback Added a test

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: MultiIndex
name: Needs Tests
description: Unit test(s) needed to prevent regressions

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-09 21:43:16 UTC
can you merge master
Done
thanks @phofl
TST: improve readability using dedent in test_to_string.py
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Improve readability of expected outputs using dedent in ``pandas/tests/io/formats/test_to_string.py``

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

Comments:
One travis job failed because of its internal error. Can you re-start?
Windows py38_np18 error seems irrelevant.
thanks @ivanovmg keep em coming
TST: move and reformat latex_na_rep test
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Move test to a more suitable location (``class TestToLatexFormatters``)
and reformat using ``_dedent`` and raw string for better readability.

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: IO LaTeX
description: to_latex

Comments:
thanks @ivanovmg
CLN: cleanup, deduplicate plotting/test_frame
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

1. Make more reasonable var names
2. Get rid of one test in favor of parametrization in another one

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Visualization
description: plotting
name: Clean

Comments:
thanks @ivanovmg very nice
Bug in loc raised Error when non-integer slice was given for MultiIndex
State: closed

- [x] closes #25165
- [x] closes #24263
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

A slice return caused issues everywhere, when this function was used. So I unpacked it right in there.

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-24 18:33:22 UTC
fixing #24263 was quite tricky. Have to return the slice here, because the codes do not equal our start value. I thinkt there is no other way as checking for slice outside of the function
@jreback im about to turn in, pls ping me on these indexing PRs to review in the AM
Hm the test I fixed was wrong previously I think
this looks fine, can you rebase and ping on green
@jreback green
thanks @phofl very nice