BrandonFors/pandas-github-issues huggingface dataset:
BUG: Use correct ExtensionArray reductions in DataFrame reductions
State: closed

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


Labels:
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
> So maybe we can edit that condition to include something like `or self._any_extension_type`.

using `self._mgr.any_extension_types` also fixes #32651 (but not checked to see if anything breaks) 
> One problem is that frame_apply path is only taken for if not self._is_homogeneous_type and filter_type is None: ....

Yah, the longer-term solution I have in mind is #34714 that all cases will go through.  Is tinkering with the is_homogeneous_type check necessary for this PR, or can we punt on that until after 1.1?
@jbrockmendel hope you don't mind, but I cherry-picked some commits with tests from me and Simon from the other PR onto this branch. Depending on whether we do the any_extension_type thing, we might need to xfail them, though

>  Is tinkering with the is_homogeneous_type check necessary for this PR, or can we punt on that until after 1.1?

I would personally prefer to do this here as well, as otherwise you get a rather surprising inconsistency which underlying operation is actually ran for DataFrames with EAs depending on wether other dtypes are present or not.

I pushed the tiny change for that as well in the last commit, to see what CI says, but we can easily undo that again.
The one failure on the py37_np18 build seems to be unrelated .. Have we seen that elsewhere as well?
> The one failure on the py37_np18 build seems to be unrelated .. Have we seen that elsewhere as well?

yep, although AFAIK we don't have an issue for this. see https://github.com/pandas-dev/pandas/pull/34906#issuecomment-652005200 and https://github.com/pandas-dev/pandas/pull/35086#issuecomment-652626570

if i see this failure on a PR, I normally just re-run.
needs whatsnew for #32651 (can copy from #34210)
@simonjayhawkins I think the general whatsnew note added by Brock already covers that as well (can maybe just add the issue number)
Merging in a few hours if no objections (cc @jreback, I think you're the only one without an explicit +1)
@jorisvandenbossche your commits look good, thanks for the heads up
thanks @jbrockmendel
BUG: ValueError on groupby with categoricals
State: closed

- [x] closes #34951 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Within `DataFrameGroupBy._cython_agg_blocks`, if it is aggregating a one-column DataFrame, it creates a `SeriresGroupBy`, calls the function on that and takes the returned values. But the `SeriesGroupBy` also does the missing-categories reindexing. The `DataFrameGroupBy` ends up with values that contain the missing categories, and an index that does not. When they are passed into a BlockManager it raises a `ValueError` stating that their lengths don't match. 

Solutions is to have `_cython_agg_blocks` create a `SeriesGroupBy` with `observed=True` so it doesn't do any reindexing. The reindexing is left to the calling `DataFrameGroupBy`

This also explains why error only occurred in `DataFrameGroupBy` but not `SeriesGroupBy`.

Labels:
name: Bug
name: Groupby
name: Categorical
description: Categorical Data Type

Comments:
Hello @smithto1! 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-07-13 20:18:46 UTC
@jreback comments addressed and all checks passing. Can you do a re-review?
very nice @smithto1 keep em coming!
REF: de-duplicate get_resolution
State: closed

This changes get_resolution to use the same less-verbose pattern as ints_to_pydatetime; hopefully we'll be able to refactor out a couple of helper functions. 

asv run says this is perf-neutral

Labels:
name: Refactor
description: Internal refactoring of code
name: Timezones
description: Timezone data dtype

Comments:
@jreback gentle ping, follow-ups in the pipeline (ditto #35359/#35356)
thanks @jbrockmendel yep back in town
TST: GH#26367 Verify whether Pandas import succeeds when setting filt…
State: closed

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


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

Comments:
Thanks for working on this, but I don't think this approach will work. Pandas is loaded in the pytest conftest, and so by the time we get to this test we've already imported pandas. Since Python caches imported modules, importing again won't re-trigger the warnings.

I think this is covered at https://github.com/pandas-dev/pandas/blob/37662ea387e17a4aca1f83445daeea977e145a9b/ci/code_checks.sh#L252, where we start a new processes and check for warnings / errors.
Thanks for the PR @avinashpancham. As Tom mentioned looks like we are already testing this, but feel free to work on any of the other issues that need regression testing!
TST: added tests for sparse and date range quantiles
State: closed

- [x] closes #24600
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Added tests for sparse and date range quantiles

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Sparse
description: Sparse Data Type
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff

Comments:
I'm new to Travis CI and based on the error I am seeing in the error log I am not understanding what is causing the issue: `requests.exceptions.ChunkedEncodingError: ("Connection broken: ConnectionResetError(104, 'Connection reset by peer')", ConnectionResetError(104, 'Connection reset by peer'))`.

Anyone have any insight into what would cause this?
thanks @sanders41
CLN/DOC: DataFrame.to_parquet supports file-like objects
State: closed

Adds documentation and type-hints for supporting file-like objects when `engine == 'pyarrow'`; relevant to #30081. Tests for this behavior currently exist in `io.test_parquet.py`:

````
@td.skip_if_no("pyarrow")
    def test_read_file_like_obj_support(self, df_compat):
        buffer = BytesIO()
        df_compat.to_parquet(buffer)
        df_from_buf = pd.read_parquet(buffer)
        tm.assert_frame_equal(df_compat, df_from_buf)
````

Perhaps the restrictions on the arguments when path is not a string:

* partition_cols must be None; and
* engine must end up being pyarrow

should be checked directly in `DataFrame.to_parquet`, but I'm leaving this out as that is an API change that could be made in a subsequent PR. The latter gives the clear error message `TypeError: expected str, bytes or os.PathLike object, not _io.BytesIO` but the former raises `AttributeError: 'NoneType' object has no attribute '_isfilestore'` which is slightly confusing.

Another API change that could be made subsequently is changing the `path` argument to `path_or_buf`, consistent with `DataFrame.to_csv`.

Labels:
name: Docs
name: IO Parquet
description: parquet, feather
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
would take a separate PR for path -> path_or_buf (needs deprecation)
Thanks @rhshadrach.
TST add test for dtype consistency with pd replace #23305
State: closed

- [x] closes #23305 
- [x] tests added / passed
- [x] passes `black pandas`
- [ ] 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:
Hello @mathurk1! 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-07-14 03:48:26 UTC
thanks @mathurk1
TST: Verify filtering operations on DataFrames with categorical Series
State: closed

- [ x ] closes #22609
- [ 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: Categorical
description: Categorical Data Type

Comments:
thanks @avinashpancham
BUG: Inconsistent behavior in Index.difference
State: closed

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


Labels:
name: Bug
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations
name: Index
description: Related to the Index class or subclasses

Comments:
Thanks @jorisvandenbossche for reviewing this. have extended an existing test
@simonjayhawkins are you able to update here?
> @simonjayhawkins are you able to update here?

i've parameterised the additional tests added in response to https://github.com/pandas-dev/pandas/pull/35231#pullrequestreview-447153446

not sure parameterise makes sense for the issue regression tests (parameterise on `order` i.e index.equals(other) and other.equals(index) or include the indexes in the parameterisation?) 

have pushed changes but not yet appearing here. 
> have pushed changes but not yet appearing here.

The changes are now showing, but ci not running.
azure and travis triggered, but not github actions. will push another empty commit later if github actions doesn't run.
lgtm. can merge on green.
TST verify return none inplace in tests/indexing
State: closed

verify we return none for all inplace calls in tests/indexing

related: https://github.com/pandas-dev/pandas/pull/35210


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

Comments:
**build failed**, it says

ci/run_tests.sh

ImportError while loading conftest '/home/travis/build/pandas-dev/pandas/pandas/conftest.py'.

ImportError: C extension: 'dtypes' from partially initialized module 'pandas._libs.tslibs' (most likely due to a circular import) (/home/travis/build/pandas-dev/pandas/pandas/_libs/tslibs/__init__.py) not built. If you want to import pandas from the source directory, you may need to run 'python setup.py build_ext --inplace --force' to build the C extensions first.

The command "ci/run_tests.sh" exited with 4.
had a similar issue locally and had to recompile everything ...
@r-toroxel Thanks for the PR. The branch has conflicts with master. can you merge upstream/master
thanks @r-toroxel
Place the calculation of mask prior to the calls of comp in replace_list to improve performance
State: closed

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


Labels:
name: Performance
description: Memory or execution speed performance
name: Strings
description: String extension data type and string data

Comments:
I've included the change in the whatsnew document of the 1.1 milestone. Do you think we can include it in that release, if this PR is approved on time? It would be nice since this change is related to https://github.com/pandas-dev/pandas/pull/32890 which is also in 1.1. 
> can you show the results of the asv's


```
       before           after         ratio
     [0ed1dcd5]       [5b6912c5]
     <master>         <improve-replace_list-performance>
-      27.2±0.8μs      24.7±0.04μs     0.91  tslibs.timestamp.TimestampAcrossDst.time_replace_across_dst
-        41.3±3μs       37.2±0.4μs     0.90  tslibs.timestamp.TimestampOps.time_replace_tz(datetime.timezone(datetime.timedelta(seconds=3600)))
-        9.03±3μs       8.02±0.1μs     0.89  tslibs.timestamp.TimestampOps.time_replace_None(tzfile('/usr/share/zoneinfo/US/Central'))
-        115±10ms         82.1±9ms     0.72  replace.Convert.time_replace('Series', 'Timedelta')
-         5.33±0s        3.59±0.1s     0.67  replace.ReplaceDict.time_replace_series(True)
-        130±20ms         85.2±8ms     0.66  replace.Convert.time_replace('Series', 'Timestamp')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.
```
thanks @chrispe92 

happy to take additional perf improvements in strings or anywhere :->
COMPAT: Ensure rolling indexers return intp
State: closed

- [x] closes #35148
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Unable to test this locally for the MacPython build, but I am pretty sure this is the main culprit of that failing job.

Labels:
name: Compat
description: pandas objects compatability with Numpy or Python functions
name: Window
description: rolling, ewma, expanding

Comments:
Wouldn't it be better if all the indexers were just platform int to begin with so we don't have to worry about casting?
no int64 are fine
Going to start over in a new branch
> @mroeschke none of this is needed or a good idea. you only need to ensure_platform_int when indexing via .take

How do we know that it's safe to cast at take time? Ensuring that indicies are the platform int seems like the way to go.
@mroeschke in case it helps, here's the dockerfile I'm using locally

```
FROM quay.io/pypa/manylinux1_i686

RUN /opt/python/cp38-cp38/bin/python -m pip install \
  cython \
  numpy \
  python-dateutil \
  pytz \
  pytest \
  hypothesis
COPY pandas /pandas
RUN cd /pandas && /opt/python/cp38-cp38/bin/python setup.py build_ext -i -j 4
```

Then run it and mount the `pandas/` directory to `/pandas` so you can easily test things.
TST: added test for groupby/apply timezone-aware with copy
State: closed

- [x] closes #27212
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

added test for groupby/apply timezone-aware with copy


Labels:

Comments:
lgtm ping on green
Thanks @jreback all checks are green.
Thanks @sanders41!
DEPR: DataFrame.lookup
State: closed

- [x] xref #18262
- [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: Deprecate
description: Functionality to remove in pandas

Comments:
This probably needs more discussion, but thought I open this PR as a starting point.
In the xref you mention "Maybe a standalone function somewhere", is this still the idea or are we going to mention `melt` and `loc`? @jreback 
no just put a nice example in the indexing docs - reference this in the deprecation
you can melt and use loc there

don’t need a standalone function
Hello @erfannariman! 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-09-16 17:58:12 UTC
@WillAyd I put `Label-based "fancy indexing"` on top, but it still fails with the same message. The message is not really informative, "warning before the summary" really does not make sense to me.
@erfannariman can you move the note to 1.2?
Yes will do, but I am still strugling with the failing tests which I'm not sure how to solve. @WillAyd 
thanks @erfannariman very nice!
TST add corner cases in test_constructors
State: closed

add two more corner cases to frame/test_constructors.


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

Comments:
you have a lint error: https://github.com/pandas-dev/pandas/pull/35216/checks?check_run_id=858699930

you can use the precommit hooks to automatically fix this.
convenient. installed.
thanks @r-toroxel
Add xarray copyright notice to comply with reuse under Apache License
State: closed

In https://github.com/xarray-contrib/pint-xarray/pull/11, @keewis and I noticed that pandas's (pandas'?...possessive forms are weird) reuse with modification of xarray code added in https://github.com/pandas-dev/pandas/commit/eee83e23ba0c5b32e27db3faca931ddb4c9619aa did not include xarray's copyright notice as required by the Apache License, Version 2.0 (which is not included in the license text itself, unlike MIT or BSD). I'm not sure how big of a deal this is, but given that pint-xarray is going ahead and modeling its code citation practice after that of pandas, I wanted to see what the thoughts are here on it. This PR takes a simple fix and just adds the copyright notice to top of the `LICENSES/XARRAY_LICENSE` file.

Also, I'm not sure if this requires a whatsnew entry or not.

- ~~closes #xxxx~~
- ~~tests added / passed~~
- ~~passes `black pandas`~~
- ~~passes `git diff upstream/master -u -- "*.py" | flake8 --diff`~~
- [ ] whatsnew entry


Labels:
name: Docs

Comments:
sure
Fix strange behavior when precision display option is zero (#20359)
State: closed

- [x] closes #20359
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Notes:
- [x] I added a unit test for the example given in #20359
- [x] As far as the context of this code is concerned, I replacde '.' with decimal string (passed from self when called) in case of non-standard decimal use


Labels:
name: Bug
name: Output-Formatting
description: __repr__ of pandas objects, to_string

Comments:
@jreback Thanks for the comments and suggestions. I'll incorporate them soon.
thanks @gimseng
Tst return none inplace series
State: closed

verify we return None for all inplace calls in /series

related: https://github.com/pandas-dev/pandas/pull/35181#event-3531612625


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

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

</samp>
thanks @r-toroxel
ENH: raise limit for completion number of columns and warn beyond
State: closed

Hi,

Currently the user-completion (ipython "\<tab\>") on a dataframe with more than 100 columns will silently ignore some columns, letting an unaware user confused on whether data disapeared.

This is actually documented, and due to an arbitrary limit set to workaround a performance issue ( See #18587 )

Dataframe with more than 100 columns are quite common, so this can potentially affect and suprise many users. Therefore, I suggest to increase that limit to, say, 1000. In any case, it would probably be good to warn the user hitting that limit. 

The attached quickfix raises the limit to 1000 and adds a warning beyond. 
(Note that I didn't experience any completion latency increasing with the axis size, so I'm not sure whether this limit is still relevant in the first place).



Labels:
name: Performance
description: Memory or execution speed performance
name: Needs Discussion
description: Requires discussion from core team before further action

Comments:
Thanks @bthyreau for the PR.

> The attached quickfix raises the limit to 1000 and adds a warning beyond.

This is still an arbitrary limit. adding a warning is maybe a good idea. Maybe this could be a pandas option to allow users to change this if required.

It maybe better to raise a feature request for more discussion on this topic.


I opened #37996 to house the discussion about this.

Thanks @bthyreau for bringing this up! We of course welcome other PRs on this (once there's a consensus on the way forward) or any other issues!
BUG: GroupBy.count() and GroupBy.sum() incorreclty return NaN instead of 0 for missing categories
State: closed

- [x] closes #31422 
- [x] closes #35028 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

*Behavioural Changes*
Fixing two related bugs: when grouping on multiple categoricals, `.sum()` and `.count()` would return `NaN` for the missing categories, but they are expected to return `0` for the missing categories. Both these bugs are fixed.

*Tests*
Tests were added in PR #35022 when these bugs were discovered and the tests were marked with an `xfail`. For this PR the `xfails` are removed and the tests are passing normally. As well, a few other existing tests were expecting `sum()` to return `NaN`; these have been updated so that the tests now expect to get `0` (which is the desired behaviour).

*Pivot*
The change to `.sum()` also impacts the `df.pivot_table()` if it is called with `aggfunc=sum` and is pivoted on a Categorical column with `observed=False`. This is not explicitly mentioned in either of the bugs, but it does make the behaviour consistent (i.e. the sum of a missing category is zero, not `NaN`). One test on test_pivot.py was updated to reflect this change. 

*Default Behaviour*
Because `df.groupby()` and `df.pivot_table()` have `observed=False` as the default, the default behaviour will change for a user calling `df.groupby().sum()` or `df.pivot_table(..., aggfunc='sum')` if they are grouping/pivoting on a categorical with missing categories. Previously the default would give them `NaN` for the missing categories, now the default will give them `0`.

What is the appropriate to highlight/document this change to the default behaviour?

Labels:
name: Groupby
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Categorical
description: Categorical Data Type

Comments:
@jreback I've made another attempt using a different approach. Check out #35241 

(I'm more thrilled with #35241 right now, so will mark this one as draft, expecting we'll decline it later.)
PERF: lookups on Timestamp attributes
State: closed

This is pretty ugly, but it avoids python-space attribute lookups e.g. `__Pyx_PyObject_GetAttrStr(((PyObject *)__pyx_v_self), __pyx_n_s_microsecond)`

cc @scoder is there a way to make this unnecessary?  I'm imagining a cdef property that aliased these attributes appropriately.

Labels:

Comments:
Hmm, looks like we still haven't documented C properties. But isn't this a use case for exactly that? We could just declare the read-only attributes as C properties on the `extern` types in `cpython/datetime.pxd`, and back them by the respective macro calls.
> But isn't this a use case for exactly that?

Maybe?  I don't think I've read about C properties.  I still get `Cdef functions/classes cannot take arbitrary decorators` when I try to decorate a cdef method with `@property`
I've written up a doc section, it'll be on readthedocs as soon as it's built:
https://cython.readthedocs.io/en/latest/src/userguide/extension_types.html#c-inline-properties

Here's the example that I've written:
https://github.com/cython/cython/blob/master/docs/examples/userguide/extension_types/c_property.pyx
Note that this is a Cython 3.0 feature.
> Note that this is a Cython 3.0 feature.

One more reason to be excited for 3.0.  Closing until then.
PR that adds properties to `datetime.pxd` welcome. :)