Skip to content

DOC: update the pandas.DataFrame.clip docstring #20212

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

Closed
wants to merge 25 commits into from
Closed
Changes from 21 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
114 changes: 78 additions & 36 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -5601,53 +5601,95 @@ def clip(self, lower=None, upper=None, axis=None, inplace=False,
"""
Trim values at input threshold(s).

Elements above/below the upper/lower thresholds will be changed to
upper/lower thresholds. Clipping data is a method for dealing with
out-of-range elements. If some elements are too large or too small,
clipping is one way to transform the data into a reasonable range.

Parameters
----------
lower : float or array_like, default None
upper : float or array_like, default None
axis : int or string axis name, optional
Align object with lower and upper along the given axis.
lower : float, array-like or None, default None
Lower threshold for clipping. Values smaller than `lower` will be
converted to `lower`.
upper : float, array-like or None, default None
Upper threshold for clipping. Values larger than `upper` will be
converted to `upper`.
axis : {0 or 'index', 1 or 'columns', None}, default None
Apply clip by index (i.e. by rows) or columns.
inplace : boolean, default False
Whether to perform the operation in place on the data
.. versionadded:: 0.21.0
.. versionadded:: 0.21.0.
Copy link
Member

Choose a reason for hiding this comment

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

I find the description of axis a bit complex.

*args, **kwargs
Additional keywords have no effect but might be accepted
for compatibility with numpy.
Copy link
Member

Choose a reason for hiding this comment

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

It was some change to the convention during the sprint, and args and kwargs should be in the same line *args, **kwargs and a common description in the next line. We didn't find any case where args and kwargs have different description, and it's repetitive the way it was originally defined.

Also, note that *args are positional arguments, I don't think it's correct to name they keywords.


Returns
-------
clipped : Series
`Series` or `DataFrame`.
Original input with those values above/below the
`upper`/`'lower` thresholds set to the threshold values.
Copy link
Member

Choose a reason for hiding this comment

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

If return can be Series, the description is not accurate, saying the DataFrame is returned. May be something like "Original input with values above/below..."?


See Also
--------
DataFrame.clip : Trim values at input threshold(s).
Series.clip : Trim values at input threshold(s).
Series.clip_lower : Return copy of the input with values below given
value(s) truncated.
Copy link
Contributor

Choose a reason for hiding this comment

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

would just mention Series.clip here (not upper/lower)

Copy link
Member

Choose a reason for hiding this comment

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

Why don't you find clip_upper and clip_lower related? I suggested them, but actually I was wondering if I was missing something, or if it was being considered deprecating them. It seems to me that clip(upper=X) could be used for clip_upper.

Copy link
Contributor

Choose a reason for hiding this comment

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

going to deprecate lower/upper but that’s not the reason
when showing a DataFrame method only like to show 1 Series method and and any related DataFrame method

just a preference to not have too many things in See Also

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense to me.

But if I'm not wrong, the title is misleading but this docstring is for the Series.clip method too.

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh ok in that case this is ok

Series.clip_upper : Return copy of input with values above given
value(s) truncated.
DataFrame.clip_lower : Return copy of the input with values below given
value(s) truncated.
DataFrame.clip_upper : Return copy of input with values above given
value(s) truncated.
DataFrame.quantile : Return values at the given quantile over requested
axis, a la numpy.percentile.

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 the prefix pandas is not needed, just Series.clip. Also, not sure if that was already discussed, but don't we want clip_lower and clip_upper here?

Copy link
Author

Choose a reason for hiding this comment

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

I think you previously mentioned that those were generic. Did we want them included still?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I think I wasn't clear enough. You should check it, but as this docstring is in generic.py I assume it'll be used by both Series and DataFrame. That's why I said it was (the docstring) was generic.

So, as your assigned docstring was DataFrame.clip (and it's what it's in the title of the PR) but you're actually working also in Series.clip, I don't think it makes sense to only add Series.clip as @jreback suggested. I'd say that we probably want also DataFrame.clip. Even if it'll be a bit weird to have in the See Also the same method which is being documented, but I think it's all right. In a separate PR we could use a template and just have the right one.

So, summarizing, the see also should contain Series.clip, DataFrame.clip, and the clip_lower and clip_upper for each of them (I assume both classes have it). And assuming that clip is commonly used with quantile as in your example (I don't know), I would also add it.

@jreback do you agree?

Copy link
Author

Choose a reason for hiding this comment

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

Ah, yes I see, I was mistaken. Sure, I can add all those.

Examples
--------
>>> some_data = {'a': [-1, -2, -100], 'b': [1, 2, 100]}
>>> df = pd.DataFrame(some_data, index = ['foo', 'bar', 'foobar'])
Copy link
Member

Choose a reason for hiding this comment

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

The spaces around = are not PEP-8 compliant. Sorry the validation script doesn't check it yet.

Copy link
Member

Choose a reason for hiding this comment

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

PEP-8 issue with spaces around =.

Also, as you need to change this, I'd directly have the dictionary in the constructor, and avoid creating some_data. Just for the sake of consistency, as all examples I've seen use this way. With one key of the dictionary in one line, and the index in a third one, I think line width should be a problem.

>>> df
0 1
0 0.335232 -1.256177
1 -1.367855 0.746646
2 0.027753 -1.176076
3 0.230930 -0.679613
4 1.261967 0.570967

>>> df.clip(-1.0, 0.5)
0 1
0 0.335232 -1.000000
1 -1.000000 0.500000
2 0.027753 -1.000000
3 0.230930 -0.679613
4 0.500000 0.500000

>>> t
0 -0.3
1 -0.2
2 -0.1
3 0.0
4 0.1
dtype: float64

>>> df.clip(t, t + 1, axis=0)
0 1
0 0.335232 -0.300000
1 -0.200000 0.746646
2 0.027753 -0.100000
3 0.230930 0.000000
4 1.100000 0.570967
a b
foo -1 1
Copy link
Contributor

Choose a reason for hiding this comment

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

alignment doesn’t look right here

bar -2 2
foobar -100 100

>>> df.clip(lower=-10, upper=10)
a b
foo -1 1
bar -2 2
foobar -10 10

You can clip each column or row with different thresholds by passing
a ``Series`` to the lower/upper argument. Use the axis argument to clip
by column or rows.

>>> col_thresh = pd.Series({'a': -5, 'b': 5})
>>> df.clip(lower=col_thresh, axis='columns')
a b
foo -1 5
bar -2 5
foobar -5 100

Clip the foo, bar, and foobar rows with lower thresholds 5, 7, and 10.

>>> row_thresh = pd.Series({'foo': 0, 'bar': 1, 'foobar': 10})
>>> df.clip(lower=row_thresh, axis='index')
a b
foo 0 1
bar 1 2
foobar 10 100

`Winsorizing <https://en.wikipedia.org/wiki/Winsorizing>`__ is a
related method, whereby the data are clipped at
Copy link
Contributor

Choose a reason for hiding this comment

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

put this reference in the Notes section

Copy link
Author

Choose a reason for hiding this comment

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

Do you want me to move the entire part about winsorization or just the link?

Copy link
Member

Choose a reason for hiding this comment

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

Just the link, you can see the format in the 13. References section of this document: http://numpydoc.readthedocs.io/en/latest/format.html

the 5th and 95th percentiles.

>>> lower, upper = df.quantile(0.05), df.quantile(0.95)
>>> df.clip(lower=lower, upper=upper, axis='columns')
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if it's just because I'm too tired, but is axis useful here? If the threshold is a scalar, doesn't seem to have an effect, right?

Copy link
Author

Choose a reason for hiding this comment

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

Axis is required here since df.quantile returns a series with the quantiles for each column. Running without the axis argument results in an error.

Copy link
Member

Choose a reason for hiding this comment

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

I'd find useful having this comment in the explanations before the test. :)

a b
foo -1.1 1.1
bar -2.0 2.0
foobar -90.2 90.2
"""
if isinstance(self, ABCPanel):
raise NotImplementedError("clip is not supported yet for panels")
Expand Down