BrandonFors/pandas-github-issues huggingface dataset:
REF: Pre-empt ValueError in _aggregate_series_fast
State: closed

cc @WillAyd @jreback orthogonal to other groupby PRs (including #29499) but will have merge conflicts.  No preference on what order they should go in.

Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
lgtm. can you merge master here and merge on green.
rebased+green; ping.
thanks @jbrockmendel
TST: add test for df.where() with int dtype
State: closed

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


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Categorical
description: Categorical Data Type

Comments:
thanks @ganevgv
TST: add test for df construction from dict with tuples
State: closed

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


Labels:
name: DataFrame
description: DataFrame data structure

Comments:
Thanks @ganevgv!
updated DataFrame.equals docstring
State: closed

- [x] closes #29408
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

Note: I tested the documentation locally and the links to assert_*_equals are working now.

Labels:
name: Docs

Comments:
Can you post a screenshot of how this appears locally? Somewhat surprised this renders the links
<img width="1198" alt="Screenshot 2019-11-09 at 11 29 31" src="https://user-images.githubusercontent.com/17853006/68527196-a73fd980-02e4-11ea-8e54-97db8f5b50b0.png">

Thanks @SaturnFromTitan
Update MultiIndex checks
State: closed

cc @WillAyd I'm holding off on most non-trivial groupby stuff, LMK if this is a problem.

Labels:
name: MultiIndex

Comments:
@jreback @WillAyd gentle ping for this and #29456.  Neither is important on its own, but both will have merge conflicts with more-important PRs on their way. 
can we completely remove _has_complex_internals? also IIRC there is an open PR about removing it (should close).
> can we completely remove _has_complex_internals? also IIRC there is an open PR about removing it (should close).

I pushed a rebase on to that this morning.  After this I'll probably have to rebase it again and thatll remove _has_complex_internals entirely.
lgtm. merge master once again as I *think* we merged something related to this in last 4 hours.
rebased+green
thxs
CLN: Removing Python 3.6 or higher references that are always true
State: closed

Follow up of #29212. After removing Python 3.5 compatibility, checks for >=py36 are always true and can be removed.

CC: @jreback 

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

Comments:
mypy complaint
This should be ready now, let me know if there is anything else.
Thanks for the clarification @simonjayhawkins, I made the change.
> This should be ready now, let me know if there is anything else.

lgtm pending https://github.com/pandas-dev/pandas/pull/29492#discussion_r344381451
Sorry, I missed that comment. Instead of updating the docstring I removed that function. With py36+ only, the function was a one liner, used just once. And it was marked as private, and used in a separate file. So, I think it was better to simply get rid of it.
not sure about this, was planning to re-use https://github.com/pandas-dev/pandas/pull/25569#issuecomment-477980938 intstead of creating custom exceptions
lgtm. can you merge master and @jbrockmendel has a comment, ping on green.
@jreback
thanks @datapythonista
CLN: annotation in reshape.merge
State: closed

This will need multiple passes, trying to keep a moderately-sized diff.

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

Comments:
@WillAyd has your comment https://github.com/pandas-dev/pandas/pull/29490#discussion_r344467468 been addressed?
@simonjayhawkins Will has given the OK, back to you
ENH: Support arrow/parquet roundtrip for nullable integer / string extension dtypes
State: closed

xref https://github.com/pandas-dev/pandas/issues/20612

This implements the `__from_arrow__` method for integer/string extension types, so roundtripping to arrow/parquet fully works.

Labels:
name: Compat
description: pandas objects compatability with Numpy or Python functions
name: IO Parquet
description: parquet, feather
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
The "red" CI is because of codecov (correctly) catching that the coverage of the changes is not good. This is because the tests only run with pyarrow master. We do not have a pyarrow dev build however (and not sure it is worth to do this for those limited number of tests, it would probably also need nightlies from pyarrow).
Ignoring coverage is fine here. Not worth setting up nightly builds against pyarrow yet.

You also have some linting failures in the docs: https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=20694
The failure on Linux py36_minimum_versions seems unrelated (an unexpected resource warning in a parser test, is this already a know failing / flaky tests?)
More feedback here? Or ready to merge?

cc @jreback
Updated to resolve conflicts. Unless more comments going to merge later today.
CLN: simplify _shallow_copy
State: closed

The class hierarchy of in/around groupby is really tough, this simplifies it a little bit

Labels:
name: Clean

Comments:
Is `GroupByMixin` used at all within groupby? Maybe worth moving the entire class
huh, looks like it isnt.  any strong opinion on where it would be moved?
Maybe merge into WindowMixin. If it’s not actually used for GroupBy I don’t even think we need to keep the GroupByMixin name as that would be rather misleading 

Sent from my iPhone

> On Nov 7, 2019, at 5:14 PM, jbrockmendel <notifications@github.com> wrote:
> 
> ﻿
> huh, looks like it isnt. any strong opinion on where it would be moved?
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or unsubscribe.

let me look 

I made these for consistency between group by and window ops
> Maybe merge into WindowMixin. If it’s not actually used for GroupBy I don’t even think we need to keep the GroupByMixin name as that would be rather misleading

For now at least I'm leaning towards keeping it where it is.  it would need to go into some shared-by-resample/window location, which describes a lot of groupby code anyway.
thxs
AreaPlot:  add support for step drawstyle
State: closed

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


Labels:
name: Visualization
description: plotting

Comments:
Hello @nick-schultz! 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-11-07 23:51:14 UTC
How are you calling this from a user perspective? Just a call to `.plot(kind="step")`? 

If you can add a test case would help frame the review - typically the most important part of any PR
> How are you calling this from a user perspective? Just a call to `.plot(kind="step")`?
> 
> If you can add a test case would help frame the review - typically the most important part of any PR

I have some more discussion in my original issue #29451 .    I think the issue stems from the fact that matplotlib has two different APIs for specifying the draw style -  fill_between uses `steps`,  where the regular line plot uses `drawstyle`. It currently would be `.plot(kind='area', step='mid')`  or `.plot.area(step='mid')` .  I'm not sure if this is the ideal user interface,  any suggestions are welcomed.

I will look into adding a test.
Can users accomplish this behavior by passing in the correct keyword arguments, which are passed through to mpl?
I don't believe so.

From what I can tell, [`AreaPlot._plot` ](https://github.com/pandas-dev/pandas/blob/6dbd2b1142a1fa1be33f031d319ee2ef7a49faef/pandas/plotting/_matplotlib/core.py#L1241) has two drawing calls, and each of them have different keywords for controlling the stepping behavior: 

1. drawing lines:
    -  `lines = MPLPlot._plot(ax, x, y_values, style=style, **line_kwds)`
    -   Keywords: **drawstyle**: {`default`, `steps`, `steps-pre`, `steps-mid`, `steps-post`}

2. filling area:
    -  `rect = ax.fill_between(xdata, start, y_values, **kwds)`
    - Keywords: **step**: {`pre`, `post`, and `mid`}


submitting the wrong keyword throws an assert, so the following does not work:

```
.plot.area(step='mid', drawstyle="steps-mid" )
```

as the first call to `MPLPlot._plot` will complain about seeing the `step` keyword

@nick-schultz is this still active? If so can you add a test case?
closing as stale, if you want to continue pls ping.
> How are you calling this from a user perspective? Just a call to `.plot(kind="step")`?

df.plot.area(drawstyle='steps-post')
API: Rename BaseGrouper.recons_codes to BaseGrouper.reconstructed_codes
State: closed

Rename ``BaseGrouper.recons_codes`` to ``BaseGrouper.reconstructed_codes``. This is an internal attribute, so ok to rename. 

Also adds some typing.

Labels:
name: Groupby
name: Clean

Comments:
this needs to coordinate with #28634 (so may need to rebase here once that is merged)
The other PR is from a first time contributor that's been at it for a month and a half, so I think let's merge that first and rebase here to keep things moving
@topper-123 if you can rebase; might need to type the added method
PR09 batch 4
State: closed

chunk of #28602 

Labels:
name: Docs

Comments:
Hello @HughKelley! 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-11-08 14:41:27 UTC
I'm unsure how to format line 133 in pandas/io/excel/_base.py. flake8 is picking up the space argument required to correctly format the list being inserted as an inappropriate indent. 
> I'm unsure how to format line 133 in pandas/io/excel/_base.py. flake8 is picking up the space argument required to correctly format the list being inserted as an inappropriate indent.

What was the issue you faced previously?
> What was the issue you faced previously?

Was adding indentation to the docstring while adding a period to a parameter description here: https://github.com/pandas-dev/pandas/pull/29466/commits/020e253c7f6eff26fa02877628e9ee4c5ec65e1a

it required changing the indentation argument to `textwrap.fill()` (inserted a list of NaN values into the string) from 4 spaces to 8 to match the rest of the string. 
Hmm can you post the built HTML doc? Looks OK on stable docs wouldn't expect to have to change the indent

https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.read_excel.html
I was adding a period and thought the indentation should match throughout the string like below but if it's ok I'll undo that change.

``` python
example = (
    """
summary

Parameters
----------
parameter_1 :  str
    Description.
"""
)
``` 
changed to 
```
example = (
    """
    summary

    Parameters
    ----------
    parameter_1 :  str
        Description.
    """
   )

@WillAyd `./scripts/validate_docstrings.py` wasn't picking up the period at the end of the `use_cols` parameter description. 

Adding an indent to the doc_string as I did in 020e253 resolved that but required adding 4 more spaces to the separator argument in `fill()` which created the `E131 hanging indent` error 
Are we talking about `usecols` or `na_values`? I think I'm confused as to the source.

In any case I think OK to tweak slightly to get it to pass validation (i.e. adding text after bullets, or something to the effect)

@datapythonista 
> Are we talking about `usecols` or `na_values`? I think I'm confused as to the source.

The original error i was trying to fix was the a missing period for `use_cols`. I fixed that by indenting the string. that created a new error in `na_values` where in order to format the list correctly I had to add spaces to the separator argument which flake8 was then picking up as a hanging indent. 
> If you want to remove the remaining `Behaves as:` and `Behavior:` that would be great, but lgtm

ah thanks
@WillAyd and @datapythonista checks are green. Thanks very much for the help on this one!
Thanks a lot for helping fix the documentation @HughKelley
Pin dateutil to 2.8.0 in requirements
State: closed

botocore has pinned dateutil to <=2.8.0, which conflicts with our dateutil here and leads to 

```
ERROR: botocore 1.13.12 has requirement python-dateutil<2.8.1,>=2.1; python_version >= "2.7", but you'll have python-dateutil 2.8.1 which is incompatible.
```

See https://github.com/boto/botocore/commit/e87e7a745fd972815b235a9ee685232745aa94f9

Closes #29465 

Labels:
name: Build
description: Library building on various platforms
name: Dependencies
description: Required and optional dependencies

Comments:
Is the error you posted from pip or conda?

Shouldn't the solver know to pick something like 2.8, which is compatible with both?
From `pip`. I don't know why the solver doesn't work. It might only work with currently installed packages rather than having collected them all together, before installing them. Anyway, the issue that I duplicated seems to say we should just wait for botocore to fix this, so I'm happy to close this PR.
I asked over in https://github.com/boto/botocore/issues/1872 when they think it'll be fixed.
i think theres consensus that this needs to be fixed in botocore, not here?
Closing, needs fix in botocore.  If I'm wrong on this, please correct me and re-open.
DEPR: is_extension_type
State: closed

It is mostly redundant with `is_extension_array_dtype`, and having both is confusing.

xref #23179.

Labels:
name: Deprecate
description: Functionality to remove in pandas

Comments:
Can this not be removed outright? It's non-public and sits behind an experimental interface, so I think not worth the effort of a deprecation cycle
i wouldnt shed any tears if we removed it altogether
I'm +1 to outright remove but let's see what @TomAugspurger and @jreback think
is this in pd.api.types?

if not ok to remove
It’s not public right? So fine to just remove.

This predated the EA interface. 

> On Nov 7, 2019, at 18:23, William Ayd <notifications@github.com> wrote:
> 
> ﻿
> I'm +1 to outright remove but let's see what @TomAugspurger and @jreback think
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.

> is this in pd.api.types?

Yes, this is in pd.api.types.  (via a `*` import)
> > is this in pd.api.types?
> 
> Yes, this is in pd.api.types. (via a `*` import)

really? it has a test that catches the earning?
We have tests (updated appropriately in this PR) for deprecated functions
in this module.

On Thu, Nov 7, 2019 at 5:29 PM Jeff Reback <notifications@github.com> wrote:

> is this in pd.api.types?
>
> Yes, this is in pd.api.types. (via a * import)
>
> really? it has a test that catches the earning?
>
> —
> 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/29457?email_source=notifications&email_token=AB5UM6CRGPV2CITZPCDWCB3QSS6GNA5CNFSM4JKBUAJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDONGXQ#issuecomment-551342942>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AB5UM6DGRZX6UENGSIFMSMDQSS6GNANCNFSM4JKBUAJQ>
> .
>

ok i see the change now, yeah have to deprecate

can u add to the master depreciation issue (or create say a 1.2 removals issue)
> can u add to the master depreciation issue (or create say a 1.2 removals issue)

will do
thanks @jbrockmendel
CLN: type annotations in groupby.grouper, groupby.ops
State: closed

@simonjayhawkins mypy is still giving a couple of complaints I could use your help sorting out:

```
pandas/core/groupby/ops.py:791: error: Signature of "groupings" incompatible with supertype "BaseGrouper"
pandas/core/groupby/ops.py:872: error: Argument 1 of "_chop" is incompatible with supertype "DataSplitter"; supertype defines the argument type as "NDFrame"
pandas/core/groupby/ops.py:884: error: Argument 1 of "_chop" is incompatible with supertype "DataSplitter"; supertype defines the argument type as "NDFrame"
```

For the groupings complaint, AFAICT the attribute has the same annotation, but in the subclass its a property instead of defined in `__init__`.  For the other two, I annotated an argument with `NDFrame` in the base class and overrode with `Series` and `DataFrame` in the subclasses.  What is the preferred idiom for this pattern?

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

Comments:
Any issues remaining here?
> Any issues remaining here?

I think should use py3.6 syntax for variable annotations going forward and would also prefer to not to see changes to .format but use f-strings instead. 
> I think should use py3.6 syntax for variable annotations going forward and would also prefer to not to see changes to .format but use f-strings instead.

3.6 annotations ill try to get in the habit of.  f-strings are going to take some getting used to
> f-strings are going to take some getting used to

just thinking it'll reduce churn as they are likely to be updated anyway. so why makes changes twice?
> just thinking it'll reduce churn as they are likely to be updated anyway. so why makes changes twice?

No reason at all.  That doesn't change the whole "I'm an old man and change will take some getting used to" thing.
rebased+green
lgtm (ex one comment), @WillAyd 
Thanks @jbrockmendel
TST: add test for empty frame groupby dtypes consistency
State: closed

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


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

Comments:
Thanks @ganevgv !
CLN: remove is_datetimelike
State: closed

- [x] closes #23914
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

AFAICT this is strictly dominated by needs_i8_conversion

Labels:
name: Datetime
description: Datetime data dtype
name: Clean

Comments:
I had a single question, else looks good.
thanks @jbrockmendel
TST: add test for indexing with single/double tuples
State: closed

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

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves

Comments:
@ganevgv can you resolve merge conflict and repush?
ENH: Add ORC reader
State: closed

- [x] closes #25229
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Added an ORC reader following the `read_parquet` API. Still need to give some additional love to the docstrings but this is at least ready for some discussion and eyes on it.

Labels:
name: IO Data
description: IO issues that don't fit into a more specific label

Comments:
Note: could also likely use some work to share the functionality between parquet and ORC readers where possible. There's a lot of copy paste currently in this PR.
I unfortunately am not going to have the bandwidth to drive this to completion likely for a while, so if someone would like to take this over it would be welcome.
ok let's see what we can do here
> Added a few comments.
> 
> For test testing files, did you write them yourself, or do they come from somewhere? I was wondering if you could make the > 50KB ones a bit smaller.

IIRC @kkraus14 mentioned these were externally generated; these are pretty small so I don't think this is a big deal (and they are skipped when we generate the stripped builds anyhow (e.g. remove tests/data)
updated
this is good to go @jorisvandenbossche @WillAyd 
@jorisvandenbossche wow I can't commit these on-line (likely because this is owned by @kkraus14 )
Thanks @kkraus14 and @jreback !

> wow I can't commit these on-line (likely because this is owned by @kkraus14 )

Strange, as the branch is open to push commits by maintainers ..
> lgtm. very nice implementation

actually this was on my office computer; we block push so likely that’s the issue; thanks for merge
Apologies that I didn't have the bandwidth to drive this to completion. Thanks @jreback and @jorisvandenbossche for driving this to completion.
thanks for putting this up @kkraus14 !
DOC: Remove errant backslashes from the Ecosystem tab on new website.
State: closed

Pandas Sprint at PyData NYC :-)
Fixed at the direction of @datapythonista 

The only file touched is a markdown file.

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
All green @jreback 
Thanks!
fixup pip env
State: closed

Closes https://github.com/pandas-dev/pandas/issues/29443

cc @MomIsBestFriend 

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

Comments:
lgtm.
@TomAugspurger Next time I have a build error, automatically tag you?
Nah. @gfyoung tagged me since I was the one who broke it :)

On Wed, Nov 6, 2019 at 4:09 PM MomIsBestFriend <notifications@github.com>
wrote:

> @TomAugspurger <https://github.com/TomAugspurger> Next time I have a
> build error, automatically tag you?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/29445?email_source=notifications&email_token=AAKAOIXCYSDLUQWLIRO7GTDQSMXBNA5CNFSM4JJ5G3O2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDH7YNA#issuecomment-550501428>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKAOISTXLZALQ4TZ6BIWVLQSMXBNANCNFSM4JJ5G3OQ>
> .
>