BrandonFors/pandas-github-issues huggingface dataset:
BUG: support corr and cov functions for custom BaseIndexer rolling windows
State: closed

- [X] closes #32865
- [X] reverts #33057
- [X] 2 tests added / 2 passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

## Scope of PR
This is the final PR needed to solve #32865. It fixes `cov` and `corr` by passing the correct argument to `rolling` constructors if the functions are called with a custom `BaseIndexer` subclass. A separate test is added for `cov` and `corr` as the previous tests were single-variable and wouldn't work with two columns of data. I also propose we revert #33057.

## Details
The reason `cov` and `corr` didn't work was that we were passing `_get_window` results (usually, window width), instead of the `BaseIndexer` subclass to `rolling` constructors called inside the function. Passing `self.window` (when necessary) fixes the issue.

The new test asserts only against an explicit array, because shoehorning a comparison against `np.cov` an `np.corrcoef` is ugly (I've tried). Our `rolling.apply` isn't really suited to be called with bivariate statistics functions, and the necessary gymnastics didn't look good to me. I don't believe the minor benefit would be worth, but please say if you think coomparing against numpy is necessary.

Thanks to @mroeschke for implementing the `NotImplemented` error-raising behavior in #33057 . Now that all the functions are fixed, I propose we revert that commit.

## Background on the wider issue
We currently don't support several rolling window functions when building a rolling window object using a custom class descended from `pandas.api.indexers.Baseindexer`. The implementations were written with backward-looking windows in mind, and this led to these functions breaking.
Currently, using these functions returns a `NotImplemented` error thanks to #33057, but ideally we want to update the implementations, so that they will work without a performance hit. This is what I aim to do over a series of PRs.

## Perf notes
No changes to algorithms.

Labels:
name: Window
description: rolling, ewma, expanding

Comments:
@jreback @mroeschke 
Final PR to solve #32865. All green, details above, please tell me what you think.
Looks like you've also committed a `doc/example.feather`. Could you remove that?
> Looks like you've also committed a `doc/example.feather`. Could you remove that?

@mroeschke 
Thanks for spotting that! Have no idea how it snuck in.
CI is being uncooperative. Probably temporary, will restart later.
All green, please take a look at the post where I recorded the results of digging into `_get_window`.
thanks @AlexKirko 

very nice work on this series! keep em coming!
IO: Fix feather s3 and http paths
State: closed

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


Labels:
name: IO Parquet
description: parquet, feather

Comments:
thanks @alimcmaster1 very nice!
DOC: Remove ambiguity in fill_value documentation
State: closed

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


Labels:
name: Docs
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
thanks @Bharat123rox
BUG, TST, DOC: fix core.missing._akima_interpolate will raise AttributeError
State: closed

- [x] closes #33426 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Wasn't sure if this warranted a whatsnew entry since its mostly code cleanup

Labels:
name: Bug
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
thanks @jreback, how's that look?
thanks @hkennyv
DOC: fix doc for crosstab with Categorical data input
State: closed

Crosstab function should set dropna=False to keep the categories which not appear in the data, but in the DOC, it seems to be inconsistent with the description. Two examples for comparison may be better for users to get this point.

https://pandas.pydata.org/docs/dev/user_guide/reshaping.html#cross-tabulations

Labels:
name: Docs
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode

Comments:
Thanks for the PR.

Looks like there are some minor fixes:

```
Check that no file in the repo contains trailing whitespaces
##[error]doc/source/user_guide/reshaping.rst:475:categories included in the cross-tabulation while setting ``dropna=False``,  <- trailing whitespaces found
```
Hi, whether I need to close this pull and create another? Thanks, this is my first PR, not very sophisticated about the process.
Oh, I find it start automatically after the push step. Thanks!
Thanks @GYHHAHA ! Could you please include a screenshot of this part of the docs? See [building the documentation](https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#building-the-documentation)

If you're in the `doc` directory, something like this should work:
```bash
python make.py clean
python make.py --single user_guide/reshaping.rst 
```
Hi, this is the screenshot. Is it OK? Thanks! @MarcoGorelli 
![pull_reshaping_doc](https://user-images.githubusercontent.com/41546976/80304841-5157e300-87eb-11ea-8813-1016a3414dd3.png)
Hi, this is the screenshot under current status. Thanks for review! @jreback 
![crosstab_cate](https://user-images.githubusercontent.com/41546976/80325176-b43a9000-8866-11ea-9f29-8bcb65b7d191.png)

lgtm long on green
Checks are down, is it ok? Thanks! @jreback 
thanks
BUG: Add warning if rows have more columns than expected
State: closed

- [x] closes #33037
- [x] tests added and passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] warning when read rows have more columns than expected

Labels:
name: API Design
name: IO CSV
description: read_csv, to_csv

Comments:
Multiple tests are not passing due to the new warning. 
Before fixing this, I want to ask if this solution is okay?
@mproszewska : Sorry for the long wait here!  Overall, the solution is on the right track.
Hello @mproszewska! 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-10-08 19:27:23 UTC
I have no idea how to check where this ipython directive error comes from.
can you rebase and ill take a look at the ipython thing
> can you rebase and ill take a look at the ipython thing

I rebased it
on the docbuild, it looks like the following is issuing a warning

```
data = "a,b,c\n4,apple,bat,\n8,orange,cow,"
pd.read_csv(StringIO(data), index_col=False)
```

under this PR, is issuing a warning here the correct thing to do?  If so, then an `:okwarning:` needs to go in io.rst on L756
> on the docbuild, it looks like the following is issuing a warning
> 
> ```
> data = "a,b,c\n4,apple,bat,\n8,orange,cow,"
> pd.read_csv(StringIO(data), index_col=False)
> ```
> 
> under this PR, is issuing a warning here the correct thing to do? If so, then an `:okwarning:` needs to go in io.rst on L756

I think so. First row has 3 values and the rest - 4. Where in in.rst should :okwarning: be added? maybe there's another way to do that. It shouldn't be a common warning.
conceptually this is ok. pls merge master and will re-look (and yes we would have to either fix the warnings or assert_produces_warning, though prob should fix the incorrect usages).
hmm this looks like overlapping with https://github.com/pandas-dev/pandas/pull/38587
closing in favor of https://github.com/pandas-dev/pandas/pull/38587
BUG: freq not retained on apply_index
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


Labels:
name: Bug
name: Frequency
description: DateOffsets

Comments:
is this a user visible change?
> is this a user visible change?

Yes, updated with whatsnew
[#33770] bug fix to prevent ExtensionArrays from crashing Series.__repr__()
State: closed

- [X] closes #33770
- [X] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

This pull request fixes a bug in `GenericArrayFormatter` that caused a crash when rendering arrays of arrays. This problem causes `Series.__repr__()` to fail with certain extension types; see #33770 for more information.

The root cause is this line:
``` python
1231       is_float_type = lib.map_infer(vals, is_float) & notna(vals)
```
The call to `notna()` here returns a 2D array if `vals` is a 2D array, while `lib.map_infer(vals, is_float)` returns a 1D array if `vals` is a 2D array.

In addition to fixing that line, this PR also adds tests for the issue.

Labels:
name: Bug
name: Output-Formatting
description: __repr__ of pandas objects, to_string
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Thanks for the prompt review!  I've pushed some updates to this branch which should hopefully address the review comments.
lgtm. running CI now. merge on green.
BUG: Adjust truncate for decreasing index
State: closed

- [x] closes #33756
- [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:
ping on green
> ping on green

@jreback Added tests and green (tests got a little verbose but couldn't think of more succinct wording) 
kk thanks
DOC: data_columns can take bool
State: closed

The ``data_columns`` argument of ``df.to_hdf`` can take ``True`` as an argument, meaning "all columns", yet the type hint is ``Optional[List[str]]``, triggering warnings in e.g. pyCharm.

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


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

Comments:
thanks!
Stratified sampling
State: closed

- [ ] Added new method stratified_sample
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:

Comments:
> Is there a particular Github issue where this feature is discussed?
> 
> Typically new features should be discussed in the Github issue with the core team + the community first.

Hi,
No, there is no github issue open for stratified sampling (or at least none
that I have found).
Reading the pandas documentation on contributing to the code, I found that
I could submit an enhancement. I thought of adding this method that
performs a stratified sampling, since there is no method that does that in
pandas.

If I have to open this discussion, how can I do so? I think users can
benefit from this method.

Thanks in advance.
In the `Issues` tab, you can click the green `New Issue` and it will direct you to create a `Feature Request` that will create an issue that everyone can discuss
> In the `Issues` tab, you can click the green `New Issue` and it will direct you to create a `Feature Request` that will create an issue that everyone can discuss

Great, I will do that. Thanks!
> In the `Issues` tab, you can click the green `New Issue` and it will direct you to create a `Feature Request` that will create an issue that everyone can discuss

Issue #33777
Thanks for your contribution, but we would rather move forward with an API that is similar to https://github.com/pandas-dev/pandas/issues/31775
REGR: fix DataFrame reduction with EA columns and numeric_only=True
State: closed

Closes #33256

For the block-wise path in `DataFrame._reduce`, we ensure to use the EA reduction for blocks holding EAs, instead of passing the EA to the `op` function (which is typically the `nanops` nan function).

This needs https://github.com/pandas-dev/pandas/pull/33758 to be merged, since that PR fixes the test that is failing here.

Labels:
name: Regression
description: Functionality that used to work in a prior pandas version
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
> is _reduce part of the interface?

yes (and the default is to raise an error saying the reduction is not supported)

> if we're avoiding nanops, can/should we make those functions either a) not handle EA cases or b) dispatch to _reduce?

Since `_reduce` is the interface that Series calls, and thus IMO also DataFrame should call for EA columsn (which I am doing here for the block wise path, and trying to do in general in #32867 (also for `numeric_only=None`), then in principle, I think we can/should always call that for EAs, and should not need to call a nanops function for an EA. Which means that in principle nanops would not need to handle EAs.

Of course, we could also decide on always calling nanops, and putting the logic to call `_reduce` for EAs in nanops. That's a choice we can make. 
(but in either way, I think the core algorithmic code of nanops should not need to handle EAs, as the `EA._reduce` can pass the correct ndarray to nanops and reinterpret the result)

More comments on this? (apart from that I need to rebase + fix the tests (update error message match))
seems ok to me
ill take another look once rebased+green, but tentatively looks good
Updated on master
(travis failure is an unrelated URLError connection error in test_read_from_http_url)
LGTM
xref #33300 to track.
REGR: disallow mean of period column again
State: closed

This reverts parts of https://github.com/pandas-dev/pandas/pull/32426 and https://github.com/pandas-dev/pandas/pull/29941 (those are only on master, not yet released), to disallow taking the mean on a Period dtype column again. The mean for period is not supported on PeriodArray itself or for a Series of period dtype (on purpose, see discussion in https://github.com/pandas-dev/pandas/pull/24757 when mean for datetimelike was added), so for DataFrame columns it should also not work. 

See also comment at https://github.com/pandas-dev/pandas/pull/32426#issuecomment-618872954

cc @jbrockmendel 

Labels:
name: Datetime
description: Datetime data dtype
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
Does this also disable median for PeriodDtype?
> Does this also disable median for PeriodDtype?

Hmm, yes, indeed. 

Now, I would say that can be left out of this PR, because:

- It's actually not even tested, and only enabled on master (no regression compared to released version)
- This PR disables it in nanops, but the follow-up PR https://github.com/pandas-dev/pandas/pull/33761 to call `EA._reduce` in `frame._reduce` will make median not work as well (even if it keeps working in nanops.nanmedian), since `PeriodArray._reduce` doesn't handle it.
- Currently the support for median is also inconsistent (xref https://github.com/pandas-dev/pandas/issues/33760), since it works for DataFrame when specifying numeric_only=False, but not for Series. Which again needs `PeriodArray._reduce` to implement median

So properly supporting median basically needs `PeriodArray._reduce` to support median, and while this is certainly something we should do, my feeling is that this is better done in a dedicated PR for that (with appropriate testing). 

@jbrockmendel does that sound OK to you? 
(and other comments on the code changes itself?)
> does that sound OK to you?

Yes, this is a Worth Doing It Right situation.
lgtm. this does not test / disable median?
See my answer to Brock above
kk great @jorisvandenbossche
BUG: MonthOffset.name
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

`is_anchored` is being accessed indirectly (its not a property).  As a result we never go down the other path, which would raise AttributeError since there is no `code_rule`.

Labels:
name: Bug
name: Frequency
description: DateOffsets

Comments:
This one should not be user-facing
CI: Revert Cython Pin (take 2)
State: closed

- [x] closes #33507

reverts #33534 

New Cython pre release available today - https://pypi.org/project/Cython/3.0a2/

cc @WillAyd 

Update: 3.0a2 (no such luck - seems there is a regression which is being looked at)

https://mail.python.org/pipermail/cython-devel/2020-April/005340.html

https://github.com/cython/cython/issues/3544

Update now trying 3.0a3 

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

Comments:
lgtm. @WillAyd @jbrockmendel any concerns?
great
TST: pd.NA TypeError in drop_duplicates with object dtype
State: closed

- [ ] closes #32992
- [ ] tests added / passed
- [ ] 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

Comments:
thanks @simonjayhawkins
BUG: Fix mixed datetime dtype inference
State: closed

- [x] closes #33741
- [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: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
is the docbuild failure real or a fluke?
> is the docbuild failure real or a fluke?

~It looks like something in user_guide/computation.rst which doesn't seem related, kicking off again to check~

Nevermind, was a typo in whatsnew that caused it
needs a rebase as well
@jreback ping
BUG: support skew function for custom BaseIndexer rolling windows
State: closed

- [X] xref #32865 
- [X] 1 tests added / 1 passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

## Scope of PR
This PR does a couple things to fix the performance of `skew`, and this also fixes the behavior of all the functions that have `require_min_periods` for small values of `min_periods`:
* clarify docs. Name all sample methods uniformly, add a note to the caveat that users should in general be careful about using sample methods with windows
* fix bug in `_apply` that made us never go into the `BaseIndexer` control flow branch
* fix bug in `_apply`: pass `window_indexer.window_size` into `calc_min_periods` instead of `min_periods or 1`. We want the custom indexer window size there, so it's not clear to me why we were passing `min_periods or 1` .

## Details
The algorithm itself is robust, but it defaults to sample skewness, which is why there was a difference between its output and `numpy`. To prevent misunderstandings, I clarified the docs a bit.
We were also passing a wrong value to `calc_min_periods`, and we weren't going into the proper if branch, because we were checking the type of `window` instead of `self.window`.

## Background on the wider issue
We currently don't support several rolling window functions when building a rolling window object using a custom class descended from `pandas.api.indexers.Baseindexer`. The implementations were written with backward-looking windows in mind, and this led to these functions breaking.
Currently, using these functions returns a `NotImplemented` error thanks to #33057, but ideally we want to update the implementations, so that they will work without a performance hit. This is what I aim to do over a series of PRs.

## Perf notes
No changes to algorithms.


Labels:
name: Window
description: rolling, ewma, expanding

Comments:
@jreback @mroeschke 
Here is the fix for `skew` with custom rolling windows. Mainly, this returns sample skewness, similarly to `std` and `var`, but I also stumbled onto a couple adjacent bugs while calling this with small values of `min_periods`.

Fixed those and edited the docs to be more clear.

Details in the description above.
Did something happen? Why are we running CI only on macOS? Can't find the issue that led to this . . .
https://github.com/pandas-dev/pandas/issues/33747
/azp run
<samp>
Azure Pipelines successfully started running 1 pipeline(s).<br>

</samp>
@jreback 
There still seem to be problems with CI. I'm reasonably confident that this fix isn't platform-dependent, but please say if you'd like to wait until we can launch all the pipelines and have them report back.

Plenty horrible stuff gets into software because 'it should have worked'.
k can you merge master again

CI should run
@jreback 
Done, all green.
thanks @AlexKirko
PERF: fix #32976 slow group by for categorical columns
State: closed

Aggregate categorical codes with fast cython aggregation for select `how` operations. Added new ASV benchmark copied from 32976 indicating > 99% improvement in performance for this case.

- [x] closes #32976
- [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: Categorical
description: Categorical Data Type
name: Stale

Comments:
> how close do you think we are to defining an API to allow passing EA values to cython arrays?
> `values = self._values_for_cython(method="first")`

If we restrict attention to ordering-based cython methods then i think this is pretty straightforward, just need to get everyone on board.
pls show the performance before / after.
latest asv results for new benchmark replicating #32976:

```
$ asv continuous HEAD -b 'groupby.CategoricalFrame'

before           after         ratio    
     [77a0f19c]       [ec70c57e]
     <master>         <cat_groupby_fix>
-      1.68±0.01s       4.51±0.1ms     0.00  groupby.CategoricalFrame.time_groupby
-      1.68±0.01s      4.43±0.09ms     0.00  groupby.CategoricalFrame.time_groupby_ordered
                                                                 
SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.          
PERFORMANCE INCREASED.
```
Hello @rtlee9! 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-08-04 01:53:13 UTC
@rtlee9 Can you merge master and fix the conflict?
Fixed conflict and merged in 7570425
@rtlee9 if you'd address the comments and merge master
@rtlee9 can you move release note to 1.2 and merge upstream/master to resolve conflict
@rtlee9 Is this still active? Some merge conflicts that need resolving.
Closing for now. @rtlee9 ping us whenever you'd like to continue and we'll reopen!