BUG: Ensure same index is returned for slow and fast path in groupby.apply #31613
Conversation
fjetter force-pushed the bugfix/fast_slow_index branch from 7169acf to ffbcbe3 Compare 
fjetter force-pushed the bugfix/fast_slow_index branch 3 times, most recently from 8838d36 to ad1ec2f Compare 
pandas/tests/groupby/test_apply.py Outdated
| tm.assert_series_equal(result, expected) | ||
| | ||
| | ||
| def test_apply_fast_slow_identical(): |
There was a problem hiding this comment.
see if we have similar fast/slow tests; place these by them (if not ok).
There was a problem hiding this comment.
There are a few tests further up the file wich affect similar code paths, e.g. test_fast_apply or test_group_apply_once_per_group which seems a suitable place
| are any of the other issues you referenced closed by this? (aside from the ones you listed) |
| is this a regression from 0.25.x? |
The issues I referenced in the issue are already closed. I primarily referenced them to highlight this as a hotspot.
I believe this is not a regression and has been around for a while now. I'll check anyhow to confirm.
|
| If this is not a regression, let's keep it for 1.1 then, I would say |
| Agreed. |
| also pls merge master |
| I've found that if I disable fast_apply, then there is a test failure in tests.groupby.test_whitelist for |
| can you merge master and ping on green and we can look |
| sorry, was a bit swamped. Will rebase and address the comments |
fjetter force-pushed the bugfix/fast_slow_index branch from ad1ec2f to ec34b4a Compare fjetter changed the title fjetter force-pushed the bugfix/fast_slow_index branch 3 times, most recently from 2187f4d to 0f6f8a2 Compare | @jreback I rebased but it seems there is a (probably unrelated) test failure on the azure pipelines with |
It is unrelated, but not yet trouble-shot. |
fjetter force-pushed the bugfix/fast_slow_index branch from 0f6f8a2 to 8503c04 Compare | I rebased on master and kept the changelog for 1.1.0 |
doc/source/whatsnew/v1.1.0.rst Outdated
| - Bug in :meth:`GroupBy.agg`, :meth:`GroupBy.transform`, and :meth:`GroupBy.resample` where subclasses are not preserved (:issue:`28330`) | ||
| - Bug in :meth:`GroupBy.rolling.apply` ignores args and kwargs parameters (:issue:`33433`) | ||
| - Bug in :meth:`DataFrameGroupby.std` and :meth:`DataFrameGroupby.sem` would modify grouped-by columns when ``as_index=False`` (:issue:`10355`) | ||
| - Bug in :meth:`core.groupby.DataFrameGroupBy.apply` where the result shape was incorrect when the output index was not identical to the input index (:issue:`31612`) |
There was a problem hiding this comment.
can you make this more user informative, e.g. what case does this happen
doc/source/whatsnew/v1.1.0.rst Outdated
| - Bug in :meth:`GroupBy.agg`, :meth:`GroupBy.transform`, and :meth:`GroupBy.resample` where subclasses are not preserved (:issue:`28330`) | ||
| - Bug in :meth:`GroupBy.rolling.apply` ignores args and kwargs parameters (:issue:`33433`) | ||
| - Bug in :meth:`DataFrameGroupby.std` and :meth:`DataFrameGroupby.sem` would modify grouped-by columns when ``as_index=False`` (:issue:`10355`) | ||
| - Bug in :meth:`core.groupby.DataFrameGroupBy.apply` where the result shape was incorrect when the output index was not identical to the input index (:issue:`31612`) |
There was a problem hiding this comment.
can you also list the other closed issue here #14927
| ping on green. |
| Trying to rewrite the changelog. pretty difficult to actually list all tickets affected by this since most users don't go into depths that far. |
what you wrote is ok, the idea is really to just list the affected tickets so that users can click thru if desired. when we close an issue we want it referenced in the whatsnew and a test to confirm :-> |
| thanks @fjetter very nice! |
8 participants
This fixes the internal check to be consistent with the slow apply path
closes API: inconsistent return format of groupby apply #13056
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff