Skip to content

Fix bug in Series.describe where the median is included any time the percentiles argument is not None #61158

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

Merged
merged 7 commits into from
Mar 21, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v3.0.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,7 @@ Other
- Bug in :meth:`DataFrame.where` where using a non-bool type array in the function would return a ``ValueError`` instead of a ``TypeError`` (:issue:`56330`)
- Bug in :meth:`Index.sort_values` when passing a key function that turns values into tuples, e.g. ``key=natsort.natsort_key``, would raise ``TypeError`` (:issue:`56081`)
- Bug in :meth:`MultiIndex.fillna` error message was referring to ``isna`` instead of ``fillna`` (:issue:`60974`)
- Bug in :meth:`Series.describe` where median percentile is included when the ``percentiles`` argument is passed (:issue:`60550`).
- Bug in :meth:`Series.diff` allowing non-integer values for the ``periods`` argument. (:issue:`56607`)
- Bug in :meth:`Series.dt` methods in :class:`ArrowDtype` that were returning incorrect values. (:issue:`57355`)
- Bug in :meth:`Series.isin` raising ``TypeError`` when series is large (>10**6) and ``values`` contains NA (:issue:`60678`)
Expand Down
9 changes: 6 additions & 3 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -10818,9 +10818,12 @@ def describe(
----------
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.
fall between 0 and 1. Here are the options:

- A list-like of numbers : To include the percentiles listed. If
that list is empty, no percentiles will be returned.
- None (default) : To include the default percentiles, which are the
25th, 50th, and 75th ones.
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 you can just keep the old description but clarify The default, ``None``, will automatically return the 25th, 50th, and 75th percentiles.

include : 'all', list-like of dtypes or None (default), optional
A white list of data types to include in the result. Ignored
for ``Series``. Here are the options:
Expand Down
11 changes: 6 additions & 5 deletions pandas/core/methods/describe.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,15 @@ def describe_numeric_1d(series: Series, percentiles: Sequence[float]) -> Series:

formatted_percentiles = format_percentiles(percentiles)

if len(percentiles) == 0:
quantiles = []
else:
quantiles = series.quantile(percentiles).tolist()

stat_index = ["count", "mean", "std", "min"] + formatted_percentiles + ["max"]
d = (
[series.count(), series.mean(), series.std(), series.min()]
+ series.quantile(percentiles).tolist()
+ quantiles
+ [series.max()]
)
# GH#48340 - always return float on non-complex numeric data
Expand Down Expand Up @@ -354,10 +359,6 @@ def _refine_percentiles(
# get them all to be in [0, 1]
validate_percentile(percentiles)

# median should always be included
if 0.5 not in percentiles:
percentiles.append(0.5)

percentiles = np.asarray(percentiles)

# sort and check for duplicates
Expand Down
3 changes: 3 additions & 0 deletions pandas/io/formats/format.py
Original file line number Diff line number Diff line change
Expand Up @@ -1565,6 +1565,9 @@ def format_percentiles(
>>> format_percentiles([0, 0.5, 0.02001, 0.5, 0.666666, 0.9999])
['0%', '50%', '2.0%', '50%', '66.67%', '99.99%']
"""
if len(percentiles) == 0:
return []

Comment on lines +1568 to +1570
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is backward-compatible as it is only extending the range of values that the input parameter can take.

percentiles = np.asarray(percentiles)

# It checks for np.nan as well
Expand Down
30 changes: 30 additions & 0 deletions pandas/tests/frame/methods/test_describe.py
Original file line number Diff line number Diff line change
Expand Up @@ -413,3 +413,33 @@ def test_describe_exclude_pa_dtype(self):
dtype=pd.ArrowDtype(pa.float64()),
)
tm.assert_frame_equal(result, expected)

@pytest.mark.parametrize("percentiles", [None, [], [0.2]])
def test_refine_percentiles(self, percentiles):
"""
Test that the percentiles are returned correctly depending on the `percentiles`
argument.
- The default behavior is to return the 25th, 50th, and 75 percentiles
- If `percentiles` is an empty list, no percentiles are returned
- If `percentiles` is a non-empty list, only those percentiles are returned
"""
# GH#60550
df = DataFrame({"a": np.arange(0, 10, 1)})

result = df.describe(percentiles=percentiles)

if percentiles is None:
percentiles = [0.25, 0.5, 0.75]

expected = Series(
{
"count": len(df.a),
"mean": df.a.mean(),
"std": df.a.std(),
"min": df.a.min(),
**{f"{p:.0%}": df.a.quantile(p) for p in percentiles},
"max": df.a.max(),
},
).to_frame(name="a")
Copy link
Member

Choose a reason for hiding this comment

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

Can you just create expected = DataFrame(...) instead of using to_frame?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the new formulation appropriate? I passed index=['a'] and had to transpose. Maybe there is a cleaner way such as to avoid the transpose?

Copy link
Member

Choose a reason for hiding this comment

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

You can do:

DataFrame(
    [len(df.a), df.a.mean(), ..., **[df.a.quantile(p) for p in percentiles], df.a.max()],
    index=pd.Index(["count", ..., **[f"{p:.0%}" for p in percentiles], ...]),
    column=["a"]
)

Copy link
Contributor Author

@MartinBraquet MartinBraquet Mar 21, 2025

Choose a reason for hiding this comment

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

Good point, although not sure it's really cleaner, as the list exhaustion is duplicated and this removes the clarity of the mapping from key to value. Is it fine as currently is, or would you rather prefer to have it as you mentioned just above?

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer the suggested method to not exercise other extraneous pandas APIs in this test like transpose or to_frame

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes total sense; will update it right now.


tm.assert_frame_equal(result, expected)
Loading