Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: RollingGroupby respects __getitem__ #35513

Merged
merged 6 commits into from
Aug 6, 2020

Conversation

mroeschke
Copy link
Member

@mroeschke mroeschke added this to the 1.1.1 milestone Aug 2, 2020
@mroeschke mroeschke added Bug Groupby Window rolling, ewma, expanding labels Aug 2, 2020
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mroeschke lgtm pending green (have restarted the failing travis job, failure was unrelated)

@@ -44,6 +44,10 @@ Bug fixes

-

**Groupby/resample/rolling**

- Bug in :class:`pandas.core.groupby.RollingGroupby` where ``__getitem__`` would not select the specified column (:issue:`35486`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe move to regression section above.

(this section is for bug fixes, we maybe should have a discussion in #35489 regarding the criteria for backporting bugfixes otherwise this section may end up empty)

@@ -15,7 +15,7 @@ including other versions of pandas.
Fixed regressions
~~~~~~~~~~~~~~~~~

-
- Bug in :class:`pandas.core.groupby.RollingGroupby` where ``__getitem__`` would not select the specified column (:issue:`35486`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit but is there a way to make this more user-friendly? Fair to say something like Bug in ... where column selection was being ignored?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, ya that phrasing would be more user friendly

df = DataFrame(
{"a": [1, 2, 3, 2], "b": [4.0, 2.0, 3.0, 1.0], "c": [10, 20, 30, 20]}
)
result = df.groupby("a")[["b"]].rolling(2).max()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also test a df.groupby('a')['b'] as well

@jreback jreback merged commit 7cf2d0f into pandas-dev:master Aug 6, 2020
@jreback
Copy link
Contributor

jreback commented Aug 6, 2020

thanks for addressing @mroeschke

@mroeschke mroeschke deleted the bug/rolling_groupby branch August 6, 2020 17:47
@simonjayhawkins
Copy link
Member

@meeseeksdev backport to 1.1.x

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Aug 6, 2020
jreback pushed a commit that referenced this pull request Aug 6, 2020
Co-authored-by: Matthew Roeschke <emailformattr@gmail.com>
@simonjayhawkins simonjayhawkins mentioned this pull request Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Groupby Window rolling, ewma, expanding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QST: is the new behavior of GroupByRolling in v1.1.0 intended?
4 participants