Skip to content

Adding testcase for quartile performed on a timestamp column group by ( closes #33168 ) #47575

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
Merged
merged 6 commits into from
Jul 3, 2022
Merged

Adding testcase for quartile performed on a timestamp column group by ( closes #33168 ) #47575

merged 6 commits into from
Jul 3, 2022

Conversation

dannyi96
Copy link
Contributor

@dannyi96 dannyi96 commented Jul 1, 2022 •
edited
Loading

Have added a testcase for quartile performed on a timestamp column group by. This change closes #33168

@pep8speaks
Copy link

pep8speaks commented Jul 1, 2022 •
edited
Loading

Hello @dannyi96! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-07-02 13:07:20 UTC

@dannyi96
Copy link
Contributor Author

dannyi96 commented Jul 1, 2022

This is my first open source PR 🙂 any & all feedback is most welcome.

Was wondering if one testcase would suffice here or we can add more by trying different inputs & its expected values ( similar to those above using @pytest.mark.parametrize )

@mroeschke
Copy link
Member

Looks pretty good. The code checks are failing though https://github.com/pandas-dev/pandas/runs/7156494708?check_suite_focus=true

@dannyi96
Copy link
Contributor Author

dannyi96 commented Jul 2, 2022

thanks @mroeschke, have corrected the code style & looks like the code checks have passed now

phofl
phofl reviewed Jul 2, 2022
View reviewed changes
pandas/tests/groupby/test_quantile.py Outdated

expected = DataFrame(
[
{"category": 6.9, "value": 206.9},
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to create the DataFrames with less data? If possible, the test should be minimal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure thing @phofl, have updated the testcase accordingly. do let me know if any other change is needed. Thanks in advance.

@jreback jreback added the Testing pandas testing functions or related to the test suite label Jul 3, 2022
@jreback jreback added this to the 1.5 milestone Jul 3, 2022
@jreback jreback added Groupby Timezones Timezone data dtype labels Jul 3, 2022
@jreback jreback merged commit d05bdeb into pandas-dev:main Jul 3, 2022
@jreback
Copy link
Contributor

jreback commented Jul 3, 2022

thanks @dannyi96

yehoshuadimarsky pushed a commit to yehoshuadimarsky/pandas that referenced this pull request Jul 13, 2022
@dannyi96 @yehoshuadimarsky
Adding testcase for quartile performed on a timestamp column group by (
b069044 
closes pandas-dev#33168 ) (pandas-dev#47575)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phofl phofl phofl left review comments

Assignees
No one assigned
Labels
Groupby Testing pandas testing functions or related to the test suite Timezones Timezone data dtype
Projects
None yet
Milestone
1.5
Development

Successfully merging this pull request may close these issues.

Quantile function fails when performing groupby on Time Zone Aware Timestamps
5 participants
@dannyi96 @pep8speaks @mroeschke @jreback @phofl