-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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: Change numeric_only default to True #46096
BUG: Change numeric_only default to True #46096
Conversation
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.
this is not going to be possible to change before 2.0
Hello @phofl, do you think this would be the way to go? I am still having some errors in the tests, when I run pytest -v pandas/tests/frame/methods/test_quantile.py
...
======== 73 passed, 8 xfailed, 6 warnings in 2.46s ========= Getting al the warnings produced by my modifcation, even though I included the next line as decorator to those tests =================== test session starts ==================== pandas/tests/frame/methods/test_quantile.py::TestDataFrameQuantile::test_numeric_only_default_false_warning PASSED ===================== warnings summary ===================== pandas/tests/frame/methods/test_quantile.py::TestDataFrameQuantile::test_quantile_axis_mixed pandas/tests/frame/methods/test_quantile.py::TestQuantileExtensionDtype::test_datelike_numeric_only[expected_data0-expected_index0-1] ../../anaconda3/envs/pandas-dev/lib/python3.8/site-packages/_pytest/cacheprovider.py:428 ../../anaconda3/envs/pandas-dev/lib/python3.8/site-packages/_pytest/stepwise.py:49 -- Docs: https://docs.pytest.org/en/stable/warnings.html
(21 durations < 0.005s hidden. Use -vv to show these durations.) |
also pls rebase |
3cffa0f
to
2a3ae84
Compare
Should we modify EXAMPLES in documentation? Lines 10573 to 10580 in 0ccad38
|
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.
yees pls update the docs to avoid showing this warning
pandas/core/frame.py
Outdated
@@ -10570,11 +10570,11 @@ def quantile( | |||
-------- | |||
>>> df = pd.DataFrame(np.array([[1, 1], [2, 10], [3, 100], [4, 100]]), | |||
... columns=['a', 'b']) | |||
>>> df.quantile(.1) | |||
>>> df.quantile(.1, numeric_only=True) |
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.
these should't be necessary, only add the option where it actually matters
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.
Done
a784237
to
f51bfca
Compare
This reverts commit 48ccac6.
5e2d02d
to
f5f7a3e
Compare
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 comments
[ | ||
pd.date_range("2014-01-01", periods=3, freq="m"), | ||
["a", "b", "c"], | ||
[DataFrame, Series, Timestamp], |
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.
What are you testing with this?
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.
The actual quantile method must ignore the non numeric columns and produce a warning. I am using three different non-numeric columns. A date column, a string column and a column of objects.
@NumberPiOso can you merge master and ping on green |
thanks @NumberPiOso |
I know I have to correct the other tests that fail due to this change. But I want confirmation that this PR is headed in the right direction before doing so.
doc/source/whatsnew/v1.5.0.rst
file if fixing a bug or adding a new feature.