-
-
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: #22899, Fixed docstring of itertuples in pandas/core/frame.py #22902
Conversation
Hello @leeyspaul! Thanks for submitting the PR.
|
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 @leeyspaul, great fixes. I added some comments that I think should make the documentation even better, and to respect some other conventions not mentioned in the issue.
pandas/core/frame.py
Outdated
@@ -894,6 +893,10 @@ def itertuples(self, index=True, name="Pandas"): | |||
The name of the returned namedtuples or None to return regular | |||
tuples. | |||
|
|||
Returns | |||
------- | |||
Returns namedtuples. |
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.
In this case instead of Returns
, can you use Yields
, as the function is a generator, and the namedtuples are yielded from it.
The exact syntax would be something like:
Yields
------
collections.namedtuple
A description of what is being yielded.
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.
Got it! Working on it now.
pandas/core/frame.py
Outdated
>>> df = pd.DataFrame({'col1': [1, 2], 'col2': [0.1, 0.2]}, | ||
index=['a', 'b']) | ||
... index=['a', 'b']) |
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.
In general I think it's better to have data that somehow looks real. I think it helps understand better, as you don't need to check in the DataFrame constructor that the row a, in the col2, has a 0.1. It's self-contained that a cat has 4 legs (see this example: https://github.com/pandas-dev/pandas/blob/master/pandas/core/frame.py#L7580).
Also, if you can also one examples with index=False
, and one with name='Animal
(or whatever concept you think it's better), that would also be valuable.
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.
Awesome. I really like the animal ideas, it is super intuitive!
I made the changes as such and will push it up now.
Examples
--------
>>> df = pd.DataFrame({'num_legs': [4,2], 'num_wings': [0,2]},
... index=['dog','hawk'])
>>> df
num_legs num_wings
dog 4 0
hawk 2 2
>>> for row in df.itertuples():
... print(row)
...
Pandas(Index='dog', num_legs=4, num_wings=0)
Pandas(Index='hawk', num_legs=2, num_wings=2)
By setting the `index` parameter to False we can remove the index
as the first element of the tuple:
>>> for row in df.itertuples(index=False):
... print(row)
...
Pandas(num_legs=4, num_wings=0)
Pandas(num_legs=2, num_wings=2)
With the `name` parameter set we set a custom name for the yielded
namedtuples:
>>> for row in df.itertuples(name='Animal'):
... print(row)
...
Animal(Index='dog', num_legs=4, num_wings=0)
Animal(Index='hawk', num_legs=2, num_wings=2)
Codecov Report
@@ Coverage Diff @@
## master #22902 +/- ##
==========================================
+ Coverage 92.18% 93.07% +0.88%
==========================================
Files 169 169
Lines 50830 58302 +7472
==========================================
+ Hits 46860 54262 +7402
- Misses 3970 4040 +70
Continue to review full report at Codecov.
|
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.
Looks really good now. Just added couple of comments, mainly formatting things.
pandas/core/frame.py
Outdated
@@ -910,17 +911,35 @@ def itertuples(self, index=True, name="Pandas"): | |||
|
|||
Examples | |||
-------- | |||
>>> df = pd.DataFrame({'col1': [1, 2], 'col2': [0.1, 0.2]}, | |||
... index=['a', 'b']) | |||
>>> df = pd.DataFrame({'num_legs': [4,2], 'num_wings': [0,2]}, |
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 add spaces after the commas? You can use flake8 --doctests frame.py
. It will generate many messages from other docstrings, but you can check the line numbers of yours, and make sure there is nothing wrong between them.
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.
Sorry totally forgot about that second test. Fixed the whitespacing between the commas.
pandas/core/frame.py
Outdated
------- | ||
Returns namedtuples. | ||
collections.namedtuple | ||
Yields namedtuples of corresponding index and column values. |
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 see what you mean here, but I think it may be difficult to understand for a beginner. May be something like Yields a namedtuple for each row in the DataFrame, which its values, and possibly the index.
.
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.
Updated the description just now! I think it's more intuitive now, please do let me know and if anything needs to be added I'll try again.
@@ -910,17 +911,35 @@ def itertuples(self, index=True, name="Pandas"): | |||
|
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.
In the See Also
, can you make the A
in the title capital? And then, the items, if you can prefix iterrows
and iteritems
with DataFrame.iterrows
...
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.
Ok, just prefixed both iterrows
and iteritems
.
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.
lgtm. Excellent docstring, I think it's very clear. Thanks for the contribution @leeyspaul, looking forward for your next one. Will merge this when checks are green.
@datapythonista awesome! Thanks for all your guidance - super helpful! I'd love to contribute more, I saw issue #22896 was still not picked up. I'd like to work on it after this gets merged if possible. |
@leeyspaul you're working on a branch, so... Just add a comment to the issue you start working on, so nobody else works on it. Thanks! |
Hey @datapythonista ! I saw that the Travis CI build failed I was wondering if this was something on my end that I need to fix. Thanks, and I'll get working on that other issue #22896 . |
They seem unrelated to your changes, a url that was not available. Travis is running again, let's see if this time we're lucky. |
Roger! :) |
Still no luck, I'm wondering what the issue is. I haven't done much with travis-ci so I'm not sure. @datapythonista Please let me know if there is a way I can help. :) |
Same thing is happening with some of my PR's. If you look in the travis log you can see this line in the feedback from the failing tests:
It looks like an issue with an url in the tests, so not related to our code. |
@leeyspaul @JustinZhengBC the problem with travis is unrelated your PRs, the issue is being tracked in #22934. Just ignore the error, we'll take care of it. |
thanks @leeyspaul |
git diff upstream/master -u -- "*.py" | flake8 --diff
#22899
I went ahead and followed the docstring conventions mentioned here:
https://pandas.pydata.org/pandas-docs/stable/contributing_docstring.html
The fixes I made were for the following errors:
Returns
section (Not sure if done correctly!)...
at line 914Please let me know if there are any issues and I will try my best to fix them ASAP.
Thank you!~