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

TST: add test for agg on ordered categorical cols #35630

Merged
merged 6 commits into from
Aug 21, 2020
Merged

TST: add test for agg on ordered categorical cols #35630

merged 6 commits into from
Aug 21, 2020

Conversation

mathurk1
Copy link
Contributor

@mathurk1 mathurk1 commented Aug 8, 2020 •
edited
Loading

@mathurk1 mathurk1 changed the title add test for agg on ordered categorical cols TST: add test for agg on ordered categorical cols @27800 Aug 8, 2020
@mathurk1 mathurk1 changed the title TST: add test for agg on ordered categorical cols @27800 TST: add test for agg on ordered categorical cols Aug 8, 2020
mroeschke
mroeschke reviewed Aug 8, 2020
View reviewed changes
pandas/tests/groupby/aggregate/test_aggregate.py Outdated
multi_index_tuple = [("nr", "min"), ("nr", "max"), ("cat_ord", "min")]
multi_index = pd.MultiIndex.from_tuples(multi_index_tuple)

data = np.array([(1, 4, "a"), (5, 8, "c")])
Copy link
Member

Choose a reason for hiding this comment

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

I think if you just pass this in as a list, then you won't have to astype() later on

@mroeschke mroeschke added the Testing pandas testing functions or related to the test suite label Aug 8, 2020
@mroeschke mroeschke added this to the 1.2 milestone Aug 8, 2020
jreback
jreback requested changes Aug 10, 2020
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

can you make sure that we are covering all of the tests in the OP as there were a number of cases. happy to enumerate them here (even though we likely have tests for most of them).

jreback
jreback requested changes Aug 19, 2020
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

this is adding a lot of code, can you simplify / parameterize things.

pandas/tests/groupby/aggregate/test_aggregate.py Outdated
@@ -1063,6 +1063,193 @@ def test_groupby_get_by_index():
pd.testing.assert_frame_equal(res, expected)


def test_groupby_single_agg_numeric_col():
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

pls add the issue number as a comment.

you don't really need the doc-string; the test name should be descriptive enough

@mathurk1
Copy link
Contributor Author

this is adding a lot of code, can you simplify / parameterize things.

Sure. Planning to add a fixture for the input dataframe and use it across the six test cases. I feel parameterizing the tests will have a big decorator function on top which might become hard to understand/maintain. Let me know your thoughts.

@jreback jreback merged commit 8bf2bc4 into pandas-dev:master Aug 21, 2020
@jreback
Copy link
Contributor

jreback commented Aug 21, 2020

thanks @mathurk1 very nice

jbrockmendel pushed a commit to jbrockmendel/pandas that referenced this pull request Aug 22, 2020
@mathurk1 @jbrockmendel
TST: add test for agg on ordered categorical cols (pandas-dev#35630)
c7f03e9
jbrockmendel pushed a commit to jbrockmendel/pandas that referenced this pull request Aug 23, 2020
@mathurk1 @jbrockmendel
TST: add test for agg on ordered categorical cols (pandas-dev#35630)
42649fb
mroeschke pushed a commit that referenced this pull request Aug 31, 2020
@jbrockmendel @mathurk1 @tkmz-n
REF: use BlockManager.apply for Rolling.count (#35883)
8f33d73 
* REF: remove unnecesary try/except

* TST: add test for agg on ordered categorical cols (#35630)

* TST: resample does not yield empty groups (#10603) (#35799)

* revert accidental rebase

* REF: use BlockManager.apply for Rolling.count

Co-authored-by: Karthik Mathur <22126205+mathurk1@users.noreply.github.com>
Co-authored-by: tkmz-n <60312218+tkmz-n@users.noreply.github.com>
jbrockmendel added a commit to jbrockmendel/pandas that referenced this pull request Aug 31, 2020
@jbrockmendel @mathurk1 @tkmz-n
REF: use BlockManager.apply for Rolling.count (pandas-dev#35883)
842d34b 
* REF: remove unnecesary try/except

* TST: add test for agg on ordered categorical cols (pandas-dev#35630)

* TST: resample does not yield empty groups (pandas-dev#10603) (pandas-dev#35799)

* revert accidental rebase

* REF: use BlockManager.apply for Rolling.count

Co-authored-by: Karthik Mathur <22126205+mathurk1@users.noreply.github.com>
Co-authored-by: tkmz-n <60312218+tkmz-n@users.noreply.github.com>
jreback pushed a commit that referenced this pull request Sep 2, 2020
@jbrockmendel @mathurk1 @tkmz-n
REF: handle axis=None case inside DataFrame.any/all to simplify _redu…
075ed8b 
…ce (#35899)

* REF: remove unnecesary try/except

* TST: add test for agg on ordered categorical cols (#35630)

* TST: resample does not yield empty groups (#10603) (#35799)

* revert accidental rebase

* REF: handle axis=None cases inside DataFrame.all/any

* annotate

* dummy commit to force Travis

Co-authored-by: Karthik Mathur <22126205+mathurk1@users.noreply.github.com>
Co-authored-by: tkmz-n <60312218+tkmz-n@users.noreply.github.com>
jreback pushed a commit that referenced this pull request Sep 2, 2020
@jbrockmendel @mathurk1 @tkmz-n
BUG: BlockSlider not clearing index._cache (#35937)
2056900 
* REF: remove unnecesary try/except

* TST: add test for agg on ordered categorical cols (#35630)

* TST: resample does not yield empty groups (#10603) (#35799)

* revert accidental rebase

* BUG: BlockSlider not clearing index._cache

* update whatsnew

Co-authored-by: Karthik Mathur <22126205+mathurk1@users.noreply.github.com>
Co-authored-by: tkmz-n <60312218+tkmz-n@users.noreply.github.com>
jreback pushed a commit that referenced this pull request Sep 2, 2020
@jbrockmendel @mathurk1 @tkmz-n
BUG: NDFrame.replace wrong exception type, wrong return when size==0 (#…
73c1d32 
…36045)

* REF: remove unnecesary try/except

* TST: add test for agg on ordered categorical cols (#35630)

* TST: resample does not yield empty groups (#10603) (#35799)

* revert accidental rebase

* BUG: NDFrame.replace wrong exception type, wrong return when size==0

* bool->bool_t

* whatsnew

Co-authored-by: Karthik Mathur <22126205+mathurk1@users.noreply.github.com>
Co-authored-by: tkmz-n <60312218+tkmz-n@users.noreply.github.com>
kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
@jbrockmendel @mathurk1 @tkmz-n
REF: use BlockManager.apply for Rolling.count (pandas-dev#35883)
f0a8b53 
* REF: remove unnecesary try/except

* TST: add test for agg on ordered categorical cols (pandas-dev#35630)

* TST: resample does not yield empty groups (pandas-dev#10603) (pandas-dev#35799)

* revert accidental rebase

* REF: use BlockManager.apply for Rolling.count

Co-authored-by: Karthik Mathur <22126205+mathurk1@users.noreply.github.com>
Co-authored-by: tkmz-n <60312218+tkmz-n@users.noreply.github.com>
kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
@jbrockmendel @mathurk1 @tkmz-n
REF: handle axis=None case inside DataFrame.any/all to simplify _redu…
e0c1639 
…ce (pandas-dev#35899)

* REF: remove unnecesary try/except

* TST: add test for agg on ordered categorical cols (pandas-dev#35630)

* TST: resample does not yield empty groups (pandas-dev#10603) (pandas-dev#35799)

* revert accidental rebase

* REF: handle axis=None cases inside DataFrame.all/any

* annotate

* dummy commit to force Travis

Co-authored-by: Karthik Mathur <22126205+mathurk1@users.noreply.github.com>
Co-authored-by: tkmz-n <60312218+tkmz-n@users.noreply.github.com>
kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
@jbrockmendel @mathurk1 @tkmz-n
BUG: BlockSlider not clearing index._cache (pandas-dev#35937)
3937b82 
* REF: remove unnecesary try/except

* TST: add test for agg on ordered categorical cols (pandas-dev#35630)

* TST: resample does not yield empty groups (pandas-dev#10603) (pandas-dev#35799)

* revert accidental rebase

* BUG: BlockSlider not clearing index._cache

* update whatsnew

Co-authored-by: Karthik Mathur <22126205+mathurk1@users.noreply.github.com>
Co-authored-by: tkmz-n <60312218+tkmz-n@users.noreply.github.com>
kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
@jbrockmendel @mathurk1 @tkmz-n
BUG: NDFrame.replace wrong exception type, wrong return when size==0 (p…
67ef906 
…andas-dev#36045)

* REF: remove unnecesary try/except

* TST: add test for agg on ordered categorical cols (pandas-dev#35630)

* TST: resample does not yield empty groups (pandas-dev#10603) (pandas-dev#35799)

* revert accidental rebase

* BUG: NDFrame.replace wrong exception type, wrong return when size==0

* bool->bool_t

* whatsnew

Co-authored-by: Karthik Mathur <22126205+mathurk1@users.noreply.github.com>
Co-authored-by: tkmz-n <60312218+tkmz-n@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

@mroeschke mroeschke mroeschke approved these changes

Assignees
No one assigned
Labels
Testing pandas testing functions or related to the test suite
Projects
None yet
Milestone
1.2
Development

Successfully merging this pull request may close these issues.

BUG: aggregation on ordered categorical column drops grouping index or crashes, depending on context
3 participants
@mathurk1 @jreback @mroeschke