-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
ENH: removed median percentile to be always included in describe GH #60550 #60557
base: main
Are you sure you want to change the base?
Conversation
- fixes pandas-dev#60550 - median percentile is default when a blank list of percentiles is passed
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.
Thanks could you add a test for this?
@mroeschke I don't think a separate test is needed as the core functionality remains the same. |
We should have had a test that when you specify Since we're changing the behavior, I think a test needs to be added. |
@mroeschke @rhshadrach thanks for the feedback, the tests are now added. |
@mroeschke, I hope you were able to look into this and merge the PR? |
Co-authored-by: Asish Mahapatra <[email protected]>
@mroeschke please do the needful. |
pandas/core/methods/describe.py
Outdated
@@ -74,7 +74,8 @@ def describe_ndframe( | |||
percentiles : list-like of numbers, optional | |||
The percentiles to include in the output. All should fall between 0 and 1. | |||
The default is ``[.25, .5, .75]``, which returns the 25th, 50th, and | |||
75th percentiles. | |||
75th percentiles.If a blank list is passed, then returns only the |
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.
Space after .
- Median percentile is only included in :meth:`~Series.describe` when a blank | ||
list is passed (:issue:`60550`). |
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 move this to v3.0.0.rst
. We should not do this breaking change in 2.3.
75th percentiles. If a blank list is passed, then returns | ||
only the 50th percentile value. |
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.
I do not think this should be the behavior. If I pass an empty list, I should not get any percentiles.
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.
I was trying to preserve the default percentile behavior. I will change this so that percentiles are omitted if a blank list is passed.
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.
I don't think we should remove the 50th percentile without at least a deprecation warning. Reporting the median by default is incredibly helpful (by default)
.describe(percentiles = [0.25])
returns 25th- and 50th-percentile #60550pandas/tests/reductions/test_describe_ndframe.py
Added type annotations to new arguments/methods/functions.NAdoc/source/whatsnew/v2.3.0.rst
file if fixing a bug or adding a new feature.