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.

Sign up for GitHub

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

Jump to bottom

Fix groupby-resample KeyError when resampling on Index and giving explicit list of columns. #50876

Merged
merged 8 commits into from
Jan 23, 2023
Merged

Fix groupby-resample KeyError when resampling on Index and giving explicit list of columns. #50876

merged 8 commits into from
Jan 23, 2023

Conversation

czroth
Copy link
Contributor

@czroth czroth commented Jan 19, 2023

@czroth
Copy link
Contributor Author

czroth commented Jan 19, 2023

Related #50841
This PR is for the main branch, #50841 is for a fix to the 1.5.x branch.

@czroth
Copy link
Contributor Author

czroth commented Jan 19, 2023

@mroeschke, here are the same changes, but on the main branch.
Let me know if there is anything else you need from me.

@czroth czroth changed the title Fix groupby-resample KeyError when resampling on Index and giving explicit list of columns. (main branch) Fix groupby-resample KeyError when resampling on Index and giving explicit list of columns. Jan 20, 2023
@czroth
Copy link
Contributor Author

czroth commented Jan 20, 2023 •
edited
Loading

There is one failing check, but from the error log I think this has nothing to do with my changes. Can anyone comment on why Ubuntu / Numpy Dev is failing? link

mroeschke
mroeschke reviewed Jan 20, 2023
View reviewed changes
Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

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

Thanks looks pretty good.

  1. Could you add a test where a list that contains a column that doesn't exist raises an appropriate error?
  2. Could you add a test where the list contains multiple columns labels that exist?

@mroeschke mroeschke added Groupby Resample resample method labels Jan 20, 2023
mroeschke
mroeschke reviewed Jan 20, 2023
View reviewed changes
pandas/tests/resample/test_resampler_grouper.py Outdated
@@ -536,3 +537,65 @@ def test_groupby_resample_size_all_index_same():
),
)
tm.assert_series_equal(result, expected)


class TestGroupByResampleTimeIndex(unittest.TestCase):
Copy link
Member

Choose a reason for hiding this comment

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

Could you keep the tests as python functions? We don't use unittest.

@czroth
Copy link
Contributor Author

czroth commented Jan 20, 2023

Sure, I'll just revert the last commit.

@czroth
Copy link
Contributor Author

czroth commented Jan 20, 2023

@mroeschke , I was typing up that question as I didn't see unittest used anywhere else in the project, but you were super quick on the review :)

@rhshadrach rhshadrach added the Bug label Jan 20, 2023
@czroth
Copy link
Contributor Author

czroth commented Jan 23, 2023

Is there anything else I should do for this PR or can I consider my portion of the work done?

@mroeschke mroeschke added this to the 2.0 milestone Jan 23, 2023
@mroeschke mroeschke merged commit 38ad5ce into pandas-dev:main Jan 23, 2023
@mroeschke
Copy link
Member

Thanks @czroth

pooja-subramaniam pushed a commit to pooja-subramaniam/pandas that referenced this pull request Jan 25, 2023
@czroth @pooja-subramaniam
Fix groupby-resample KeyError when resampling on Index and giving exp…
75ed44e 
…licit list of columns. (pandas-dev#50876)

* Add failing test reproducing groupby-resample KeyError (pandas-dev#50840)

* Fix groupby-resample KeyError (pandas-dev#50840) by adding None check.

* Update whatsnew for pandas-dev#50840

* Improve coverage with multi and missing column groupby-resample tests.

* Refactor groupby-resample tests via TestCase to remove duplicate code.

* Revert "Refactor groupby-resample tests via TestCase to remove duplicate code."

This reverts commit d522606.

* Fix typo in bug fix entry for pandas-dev#50840 in doc/source/whatsnew/v2.0.0.rst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroeschke mroeschke mroeschke approved these changes

Assignees
No one assigned
Labels
Bug Groupby Resample resample method
Projects
None yet
Milestone
2.0
Development

Successfully merging this pull request may close these issues.

BUG: df.groupby().resample()[[cols]] raise KeyError when resampling on index
3 participants
@czroth @mroeschke @rhshadrach