BrandonFors/pandas-github-issues huggingface dataset:
BUG/API: SeriesGroupBy reduction with numeric_only=True
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry

This addresses the SeriesGroupBy (easier) half of #41291:

1) When we have numeric_only=True and non-numeric data in a SeriesGroupBy reduction, raise NotImplementedError (matching Series behavior) instead of falling back to an often-wrong fallback (xref #41341)
2) When the user doesn't explicitly pass `numeric_only=True`, change the default to False for SeriesGroupBy, leaving DataFrameGroupBy unaffected.

Labels:
name: Bug
name: Groupby
name: API Design

Comments:
if you can rebase this one
haven't looked yet
Rebased+green
@jreback i think comments have been addressed; this is a blocker for fixing the analogous DataFrameGroupBy behavior
Thanks @jbrockmendel
DEP: bump pyarrow to 1.0.0
State: closed

- [x] closes #41329
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry


Labels:
name: Dependencies
description: Required and optional dependencies

Comments:
Closing based in Joris comment.
CI: Upgrade "actions/(checkout|cache)" to version 2
State: closed

According to the change log of both "checkout" and "cache" this should increase performance.

---

Fell free to close this PR if this was already discussed, I haven't saw any discussion about this topic.

Labels:
name: CI
description: Continuous Integration

Comments:
@ShaharNaveh do you have time to have a look at the comment and leave the CI green?
> @ShaharNaveh do you have time to have a look at the comment and leave the CI green?

OFC:)
Checked quickly, and this does seem to speed up the checkout a bit, from around 28 second to around 22 (not sure if very significant, but nice to use the latest version of the action regardless).

What I see is that if we don't fetch all tags and branches the checkout is almost immediate (like 2 seconds). I think we need them for the release notes, but I'm not aware of anything else that should require them. @ShaharNaveh would you mind having a look and see if the `fetch-depth` is required for all builds. I don't think anything other than the docs build should need them.

Also, if you want to have a better look at timings, including the ones of the cache action (I didn't check those), that would be great.
> Checked quickly, and this does seem to speed up the checkout a bit, from around 28 second to around 22 (not sure if very significant, but nice to use the latest version of the action regardless).
> 
> What I see is that if we don't fetch all tags and branches the checkout is almost immediate (like 2 seconds). I think we need them for the release notes, but I'm not aware of anything else that should require them. @ShaharNaveh would you mind having a look and see if the `fetch-depth` is required for all builds. I don't think anything other than the docs build should need them.

@datapythonista The way I decided where each `fetch-depth: 0` , was to look at the failed tests in https://github.com/pandas-dev/pandas/pull/41336/commits/e35cc1be12814e59b5ce67e2764ce7e1bddb0923 and I added the `fetch-depth: 0` wherever there was a failing test.

> Also, if you want to have a better look at timings, including the ones of the cache action (I didn't check those), that would be great.

I did see an improvement with the cache, but the difference is `1s`.
But the `v2` of the cache action is handling the cache with the `zstd` compression, which means that the cache size can be larger, so I do expect to get an improvement overtime, as the cache will get larger and larger.


Thanks @ShaharNaveh 
We've had several failures in the last couple of days, could there be a compatibility issue with conda-incubator/setup-miniconda@v2 or maybe this was failing before and I didn't notice.
> We've had several failures in the last couple of days, could there be a compatibility issue with conda-incubator/setup-miniconda@v2 or maybe this was failing before and I didn't notice.

I saw a connection error on conda, are those the failures you are referring to? We can revert, but probably better to try to understand the problem better first.
ENH: Add dropna argument to DataFrame.value_counts()
State: closed

- [x] closes #41325
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry


Labels:
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff
name: API - Consistency
description: Internal Consistency of API/Behavior

Comments:
Two of the builds failed due to tests that I have not touched. It seems like those tests fail quite often?
Is it possible to rerun those builds?
thanks @connesy very nice

That CI build fails occasionaly and is unrelated here, so all good.
[ArrowStringArray] PERF: use pa.compute.match_substring_regex for str.fullmatch if available
State: closed


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

Comments:
```
[ 50.00%] ··· strings.Methods.time_fullmatch                                                                                                              ok
[ 50.00%] ··· ============== ==========
                  dtype                
              -------------- ----------
                   str        27.6±0ms 
                  string      22.1±0ms 
               arrow_string   2.35±0ms 
              ============== ==========
```

object fallback
```
[ 50.00%] ··· strings.Methods.time_fullmatch                                                                                                              ok
[ 50.00%] ··· ============== ==========
                  dtype                
              -------------- ----------
                   str        26.6±0ms 
                  string      22.1±0ms 
               arrow_string   27.0±0ms 
              ============== ==========
```


thanks !
REF: dont special-case ngroups==0
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [ ] whatsnew entry


Labels:
name: Groupby

Comments:
can you rebase
rebased + green
[ArrowStringArray] CLN: remove hasattr checks
State: closed

xref https://github.com/pandas-dev/pandas/pull/41281#discussion_r625926407

Labels:
name: Strings
description: String extension data type and string data
name: Clean

Comments:
one more rebase if you would (just merged another arrow string PR )
> lgtm merge on green

only codecov/patch failing so merging
[ArrowStringArray] PERF: use pa.compute.match_substring_regex for str.match if available
State: closed


```
[ 50.00%] ··· strings.Methods.time_match                                                                                                                  ok
[ 50.00%] ··· ============== ==========
                  dtype                
              -------------- ----------
                   str        28.3±0ms 
                  string      22.5±0ms 
               arrow_string   2.46±0ms 
              ============== ==========
```

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

Comments:
thanks @simonjayhawkins
BUG: groupby(axis=0).rank(axis=1)
State: closed

- [x] closes #41320
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry


Labels:
name: Bug
name: Groupby

Comments:
does this cloe https://github.com/pandas-dev/pandas/issues/38278 as well?
no
thxs
REF: remove no-op casting
State: closed

In _python_agg_general:

```
            if self.grouper._filter_empty_groups:
                mask = counts.ravel() > 0
```

It isn't entirely trivial, but we can show that we will always have `mask.all()`, as a result of which the remainder of this code chunk of code is a no-op.  This PR removes it.

As a consequence, we can remove `_filter_empty_groups` and `compressed`


Labels:
name: Refactor
description: Internal refactoring of code
name: Groupby

Comments:
nice!
[ArrowStringDtype] Make it already a StringDtype subclass
State: closed

@simonjayhawkins this will be redundant after https://github.com/pandas-dev/pandas/pull/39908, but in the meantime, ArrowStringDtype being a subclass would help for writing robust code related to `StringDtype` in downstream packages (for those testing against pandas master)

Labels:
name: Strings
description: String extension data type and string data

Comments:
Thanks @jorisvandenbossche lgtm

we could clean-up the various `isinstance(..., StringDtype)` -> `isinstance(..., (StringDtype, ArrowStringDtype))` checks. do that here or as a follow-up?
> we could clean-up the various `isinstance(..., StringDtype)` -> `isinstance(..., (StringDtype, ArrowStringDtype))` checks. do that here or as a follow-up?

Did that here
```
pandas/core/arrays/string_arrow.py:113: error: Return type "Type[ArrowStringArray]" of "construct_array_type" incompatible with return type "Type[StringArray]" in supertype "StringDtype"  [override]
pandas/core/arrays/string_arrow.py:129: error: Return type "ArrowStringArray" of "__from_arrow__" incompatible with return type "StringArray" in supertype "StringDtype"  [override]
```
@simonjayhawkins is there an easy solution for that? (sorry, currently don't have time to dive into this, otherwise will add some ignores)
in #39908 we add ArrowStringArray to the return types and import it in a TYPE_CHECKING block
Would you be fine with ignoring it for now, and then properly fixing it in #39908 ? 
I could copy the approach from #39908, but that also seems a bit strange to do right now since the StringDtype itself doesn't yet return `ArrowStringArray`.
sure
REF: share GroupBy.transform
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [ ] whatsnew entry


Labels:
name: Refactor
description: Internal refactoring of code
name: Groupby

Comments:
looks fine @mroeschke quick glance if you would.
Deprecate inplace in Categorical.set_categories.
State: closed

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


Labels:
name: Categorical
description: Categorical Data Type
name: Deprecate
description: Functionality to remove in pandas

Comments:
thanks @OlehKSS
ASV: add benchmarks to test formatting func in Styler.
State: closed

The main time in rendering Styler is consumed by:

- formatting values for display
- adding cell styles via apply / applymap
- adding cell styles via set_td_classes

We have benchmarks for the latter 2 but not the first. Any future changes to the formatting function might be worth having benchmarks (I would use them :) )
Especially if we try to merge Styler formatting with the floatformatter for example. 


Labels:
name: Benchmark
description: Performance (ASV) benchmarks

Comments:
thanks
REF: raise more selectively in libreduction
State: closed

This allows slightly more cases to go through the cython path

Labels:
name: Refactor
description: Internal refactoring of code
name: Reduction Operations
description: sum, mean, min, max, etc.

Comments:
i suppose then perf diff?
> i suppose then perf diff?

presumably.  havent measured since its such a tiny corner case thats affected (1 test)
REF: support object dtype in libgroupby.group_add
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [ ] whatsnew entry


Labels:
name: Groupby

Comments:
does this change any tests?
> does this change any tests?

no, just avoids going through fallback paths
> > does this change any tests?
> 
> no, just avoids going through fallback paths

kk so in theory more performant? (merging regardless as this is a good idea)
BTW the biggest blocker among the groupby stuff that your attention could help unblock is #41291
BUG: fix suppressed TypeErrors in Groupby.mean, median, var
State: closed

These are raising TypeError because we aren't passing enough args, but then we're catching those TypeErrors and falling back.  So this doesn't change behavior, just avoids an unnecessary fallback in some cases.

No tests.  Surfacing the exceptions would require ugly/invasive changes.

Labels:
name: Groupby

Comments:
does this change perf?
> does this change perf?

haven't measured, mostly object-dtype is affected
> > does this change perf?
> 
> haven't measured, mostly object-dtype is affected

kk great. (also if you can make sure this is fully tested with object dtype as well)
Bug in iloc.setitem orienting IntegerArray into the wrong direction
State: closed

- [x] closes #40933
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them

This is more or less a follow up of #39040, so don't think we need a whatsnew?

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: NA - MaskedArrays
description: Related to pd.NA and nullable extension arrays

Comments:
thanks @phofl very nice as always
Thanks @jreback
REF: Avoid fallback in GroupBy._agg_general
State: closed

pre-emptively use non-cython path in explicit cases instead of try/except.  Many of the affected cases involve empty DataFrames and I think the tested behavior is sketchy (e.g. the fallback paths ignore the `numeric_only` kwarg), so putting this in draft mode while I look at that.

@WillAyd can i get your help with _wrap_applied_output here?  As commented, the added case is pretty much a kludge and I think we can do better.  Am I remembering correctly that you refactored a bunch of the wrapping code ~18 months ago?

Labels:
name: Groupby

Comments:
Yea a lot of these paths are thorny. I did take a look quite some time ago but never made it to far.

+1 on what you are trying to do here - I really like the explicitness
Better alternatives are in the pipeline; closing.
DOC: Fix docs in pandas/io/excel/*
State: closed

- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them


Labels:
name: Docs
name: IO Excel
description: read_excel, to_excel

Comments:
thanks @ShaharNaveh
TYP: core.sorting
State: closed

both mypy and i have a hard time with `labels, lshape = map(list, zip(*map(maybe_lift, labels, lshape)))`

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

Comments:
> both mypy and i have a hard time with `labels, lshape = map(list, zip(*map(maybe_lift, labels, lshape)))`

I just tried
```python
zip(*(maybe_lift(label, shape_) for label, shape_ in zip(labels, shape)))
```
and it seems to work:

```diff
diff --git a/pandas/core/sorting.py b/pandas/core/sorting.py
index ee62254052..dc4c21a3e4 100644
--- a/pandas/core/sorting.py
+++ b/pandas/core/sorting.py
@@ -147,9 +147,7 @@ def get_group_index(labels, shape: Shape, sort: bool, xnull: bool):
     if not xnull:
         # error: Incompatible types in assignment (expression has type
         # "List[_T]", variable has type "List[int]")
-        labels, lshape = map(  # type: ignore[assignment]
-            list, zip(*map(maybe_lift, labels, lshape))
-        )
+        labels, shape = zip(*(maybe_lift(label, shape_) for label, shape_ in zip(labels, shape)))
 
     labels = list(labels)
```
```console
$ mypy pandas
Success: no issues found in 1283 source files
```
`pandas/tests/test_sorting.py ` still passes too
@MarcoGorelli is there a convenient way to get rid of the "*"?  this feels like the kind of thing where the answer is hitting me in the face...
Could define `unzip` somewhere as suggested here https://stackoverflow.com/a/22115957 . Might make it less mysterious than `zip(*`
DOC: Fix docs for io/json/*
State: closed

- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them


Labels:
name: Docs
name: IO JSON
description: read_json, to_json, json_normalize

Comments:
thanks @ShaharNaveh
CI: Added unselected directories to doctest CI
State: closed

Some of those do have docstrings with example to be validated, and some do not have.

This only make sure that future files or changes to files in those directories will be checked.

---

- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them


Labels:
name: CI
description: Continuous Integration

Comments:
great ping when green.
thanks @ShaharNaveh
TST: fix casting extension tests for external users
State: closed

cc @simonjayhawkins fixtures defined by pandas are not known when inheriting the tests as external package. There are a bunch of fixtures that external packages are expected to define in `pandas/tests/extension/conftest.py`, but I think for the string dtype here this is not worth it to expect downstream users to define it themselves (certainly given that it is still going to change)

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Strings
description: String extension data type and string data

Comments:
Failure is the flaky ResourceWarning
Allow to union `MultiIndex` with empty `RangeIndex`
State: closed

- [x] closes #41234
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry

See also https://github.com/dask/dask/issues/7610 and https://github.com/JDASoftwareGroup/kartothek/issues/464. The regression seems to have been introduced in  pandas-dev/pandas#38671.

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

Comments:
thanks @mlondschien
REF: consolidate casting in groupby agg_series
State: closed

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [ ] whatsnew entry


Labels:
name: Refactor
description: Internal refactoring of code
name: Groupby

Comments:
These tests are for the low-level cython func, and this PR makes the
casting from object take place after the cython call instead of within it.
Nothing user facing.

On Mon, May 3, 2021 at 8:17 AM Jeff Reback ***@***.***> wrote:

> ***@***.**** requested changes on this pull request.
> ------------------------------
>
> In pandas/tests/groupby/test_bin_groupby.py
> <https://github.com/pandas-dev/pandas/pull/41273#discussion_r625159891>:
>
> > @@ -20,7 +20,7 @@ def test_series_grouper():
>      grouper = libreduction.SeriesGrouper(obj, np.mean, labels, 2)
>      result, counts = grouper.get_result()
>
> -    expected = np.array([obj[3:6].mean(), obj[6:].mean()])
> +    expected = np.array([obj[3:6].mean(), obj[6:].mean()], dtype=object)
>
> these look like internal tests, but is anything user facing changed? (no
> looks like)?
>
> why are these now returning object? (e.g. can you just remove the
> cast_back above)?
>
> —
> 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/41273#pullrequestreview-650449427>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AB5UM6EBWNTRNKVI3MBDDBLTL25AHANCNFSM437ZXAQA>
> .
>
REF: simpilify _cython_agg_general
State: closed

- make the exception-handling in SeriesGroupBy._cython_agg_general match that in DataFrameGroupBy._cython_agg_general
- remove an unnecessary for loop
- define array_func to match the pattern in DataFrameGroupBy._cython_agg_general in the hopes that we can share these methods before long

Labels:
name: Refactor
description: Internal refactoring of code
name: Groupby

Comments:
great, keep up the de-tangling!
CI: fix test for SSL warning
State: closed

follow up to #41083 to change test

Labels:
name: CI
description: Continuous Integration

Comments:
any idea why str(actual_warning) doesnt work here?
> any idea why str(actual_warning) doesnt work here?

No clue.  But I looked at output from one of my CI runs and saw that the message is what contains the string
thanks @Dr-Irv
ENH: make `Styler` compatible with non-unique indexes
State: closed

- [x] closes #41143 

The PR aims to make Styler compatible with non-unique indexes/columns, for the purpose of rendering all DataFrame types (even if no styling is applied)

- [x] `Styler.format`: made FULLY compatible with some modifications to the loops, inc TESTS.
- [x] 
`Styler.apply` and `Styler.applymap`: made PARTIALLY compatible:
    -  if `subset`s are unique then compatible, inc TESTS.
    - if `subset`s are non-unique slices will raise a not compatible `KeyError`, inc. TESTS

- [x] `Styler.apply` and `applymap` are NOT compatible. Raises KeyError.
- [x] `Styler.set_table_styles`: made FULLY compatible and will style multiple rows/columns from a non-unique key, inc TESTS.
- [x] `Styler.set_td_classes` uses `reindex` so is PARTIALLY compatible where `classes` has unique index/columns: now returns a `KeyError` in non-unique case, inc TESTS. 
- [x] `Styler.set_tooltips` uses `reindex` so is PARTIALLY compatible where `ttips` has unique index/columns: now returns a `KeyError` in non-unique case, inc TESTS. 
- [x] `Styler.hide_index` and `.hide_columns` are already FULLY compatible through existing code (inc TESTS)
- [x] all the `built-in` styling functions use some version of `apply` or `applymap` so are captured by the above cases.



I believe this is all relevant functionality reviewed.




Labels:
name: Styler
description: conditional formatting using DataFrame.style

Comments:
ok!
Bug in setitem raising ValueError with row-slice indexer on df with list-like on rhs
State: closed

- [x] closes #40440
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry

I'll adress the IntegerArray case from #40933 after this is merged

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

Comments:
thanks @phofl
ENH: Numba engine for EWM.mean
State: closed

- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry


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

Comments:
thanks @mroeschke