-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
DOC: update the docstring for several functions and properties (Seoul). #20099
Conversation
pandas/core/frame.py
Outdated
@@ -2432,7 +2432,8 @@ def eval(self, expr, inplace=False, **kwargs): | |||
return _eval(expr, inplace=inplace, **kwargs) | |||
|
|||
def select_dtypes(self, include=None, exclude=None): | |||
"""Return a subset of a DataFrame including/excluding columns based on | |||
""" | |||
Return a subset of a DataFrame including/excluding columns based on |
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.
Let's shorten this a bit so that it fits on a line. How about
Return a subset of the DataFrame based on the column dtypes.
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 for your comments. I've got one question about docstring guideline.
I looked through several comments left on other PRs and found this type of docstring is correct.
"""This type of docstring.
I ran the script mentioned in the guideline but it said docstring should be start with new line like this.
"""
This type of docstring.
Which one is correct one?
Thanks.
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.
It's the second one, so on new line (we changed that in the docstring guie rather late before the sprint)
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.
@jorisvandenbossche Thanks. I've made docstrings based on the second one and changed things you and other contributors pointed out
Can you try to fix the failing examples? |
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.
Thansk! FYI, best to do one docstring at a time. Makes it easier to review.
pandas/core/generic.py
Outdated
@@ -4232,7 +4232,8 @@ def as_matrix(self, columns=None): | |||
|
|||
@property | |||
def values(self): | |||
"""Numpy representation of NDFrame | |||
""" | |||
Return NDFrame as ndarray or ndarray-like depending on the 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.
I think #20065 was working on .values
. That has more comments so maybe remove your changes here.
pandas/core/generic.py
Outdated
@@ -4260,16 +4271,76 @@ def _get_values(self): | |||
return self.values | |||
|
|||
def get_values(self): | |||
"""same as values (but handles sparseness conversions)""" | |||
""" | |||
Same as values (but handles sparseness conversions). |
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.
Let's avoid reference to .values in the first line.
Return a NumPy representation of the data after converting sparse to dense.
And then we can use the See Also
to mention .values
pandas/core/generic.py
Outdated
|
||
Returns | ||
------- | ||
numpy.ndaray |
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.
ndarray
(two r
s)
pandas/core/generic.py
Outdated
... 'b': [True, False], 'c': [1.0, 2.0]}) | ||
>>> df.get_values() | ||
array([[0.25209328532218933, True, 1.0], | ||
[0.35383567214012146, False, 2.0]], dtype=object) |
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 this formatting is quite right. Can you try using non-random data and running the doctest on it?
pandas/core/generic.py
Outdated
|
||
Returns | ||
------- | ||
dtype Number of 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.
colon between parameter name and the type.
Also I thikn it should be
`dtype : Series
Series with the count of columns with each dtype
pandas/core/generic.py
Outdated
-------- | ||
>>> a = [['a', 1, 1.0], ['b', 2, 2.0], ['c', 3, 3.0]] | ||
>>> df = pd.DataFrame(a, columns=['str', 'int', 'float']) | ||
>>> df['int'].astype(int) |
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.
If you make df
as from a dictionary the types should be correctly inferred and you won't need astypes
pandas/core/generic.py
Outdated
|
||
Returns | ||
------- | ||
dtype Number of dtype:dense|sparse |
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.
colon. Same comment as above.
pandas/core/generic.py
Outdated
Returns | ||
------- | ||
numpy.ndaray | ||
Numpy representation of NDFrame |
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.
We should not mention "NDFrame" in public docstrings (see the output of the validation script). In this case, this can just be "DataFrame" (because Series has its own implementation and docstring)
This will also need a rebase on master |
Codecov Report
@@ Coverage Diff @@
## master #20099 +/- ##
=========================================
Coverage ? 91.7%
=========================================
Files ? 150
Lines ? 49149
Branches ? 0
=========================================
Hits ? 45071
Misses ? 4078
Partials ? 0
Continue to review full report at Codecov.
|
ad2923c
to
9ad55ec
Compare
Thanks @coffeedjimmy ! |
Checklist for the pandas documentation sprint (ignore this if you are doing
an unrelated PR):
scripts/validate_docstrings.py <your-function-or-method>
git diff upstream/master -u -- "*.py" | flake8 --diff
python doc/make.py --single <your-function-or-method>
Please include the output of the validation script below between the "```" ticks:
If the validation script still gives errors, but you think there is a good reason
to deviate in this case (and there are certainly such cases), please state this
explicitly.
-> Most of them occur because of missing extended summaries. Functions and properties I added docstrings are fairly well explained without extended summaries I think.
Some of the examples are failed because I used random functions for several examples. This makes different results in each execution. Also, I omitted some outputs because of its simplicity and clearness.
Lastly, I left errors already occurred in the previous version without changes.