-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: GroupBy.quantile fails with pd.NA #43150
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
Conversation
debnathshoham
commented
Aug 21, 2021
- closes BUG: GroupBy's quantile incompatible with pd.NA #42849
- tests added / passed
- Ensure all linting tests pass, see here for how to run them
- whatsnew entry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small comment, ping on green
@jreback @jbrockmendel Green |
Hi @jreback , @jbrockmendel - could you please take a look if this looks fine |
|
||
def test_groupby_quantile_allNA_column(): | ||
# GH#42849 | ||
df = DataFrame({"x": [1, 1], "y": [pd.NA] * 2}, dtype="Float64") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason for this to be Float64 instead of any_foo_dtype?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what any_foo_dtype
means.
In case it means why the explicit dtype? then without explicitly defining with nullable dtypes, the columns becomes object
, and quantile fails (maybe I am wrong, but I believe this is the expected behaviour). Below on master.
In [3]: import pandas as pd
In [4]: pd.__version__
Out[4]: '1.4.0.dev0+540.ga826be1f61'
In [5]: DataFrame({"x": [1, 1], "y": [pd.NA] * 2}).dtypes
Out[5]:
x int64
y object
dtype: object
In [6]: DataFrame({"x": [1, 1], "y": [pd.NA] * 2}, dtype=float).dtypes
<ipython-input-6-4731f064b6c6>:1: FutureWarning: Could not cast to float64, falling back to object. This behavior is deprecated. In a future version, when a dtype is passed to 'DataFrame', either all columns will be cast to that dtype, or a TypeError will be raised.
DataFrame({"x": [1, 1], "y": [pd.NA] * 2}, dtype=float).dtypes
Out[6]:
x float64
y object
dtype: object
def test_groupby_quantile_NA_float(any_float_dtype): | ||
# GH#42849 | ||
df = DataFrame({"x": [1, 1], "y": [0.2, np.nan]}, dtype=any_float_dtype) | ||
result = df.groupby("x")["y"].quantile(0.5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you do a case with a listlike qs e.g. [0.5, 0.75]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added below
Hi @jbrockmendel could you pls take a quick look if this is fine now? |
tm.assert_series_equal(expected, result) | ||
|
||
result = df.groupby("x").quantile(0.5) | ||
expected = DataFrame({"y": 3.5}, index=Index([1], name="x")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: clearer to just do expected = expected["y"]
next time
LGTM wouldnt surprise me if some of the tests could be condensed/parametrized, OK for follow-up |
thanks @debnathshoham (followup to parameterize tests here would be great, that can be on master) |