DOC: Fixed the doctsring for _set_axis_name (GH 22895) #22969

bkjchoi72 · 2018-10-03T09:08:31Z

Fixed the docstring of the function _set_axis_name in pandas/core/generic.py

Closes #22895

pep8speaks · 2018-10-03T09:08:34Z

Hello @bkjchoi72! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/core/generic.py !

Comment last updated on October 09, 2018 at 08:34 Hours UTC

codecov · 2018-10-05T03:16:16Z

Codecov Report

Merging #22969 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@ Coverage Diff @@ ## master #22969 +/- ## ========================================== + Coverage 92.28% 92.28% +<.01%  ========================================== Files 161 161 Lines 51451 51452 +1 ========================================== + Hits 47483 47484 +1  Misses 3968 3968

Flag	Coverage Δ
#multiple	`90.68% <100%> (ø)`	⬆️
#single	`42.28% <100%> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/generic.py	`96.84% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update deb7b4d...9d944e8. Read the comment docs.

gfyoung

Nice!

cc @datapythonista

gfyoung · 2018-10-06T22:42:15Z

@bkjchoi72 : FYI, you don't have to close your PR (#22968) and open a new if there are issues with it. You can keep making commits to the existing PR and fix them there.

pandas/core/generic.py

jreback · 2018-10-07T22:26:38Z

pandas/core/generic.py

+ The axis to set the label. The value 0 or 'index' specifies index,
+ and the value 1 or 'columns' specifies columns.
+ inplace : bool, default False
+ Whether to modify `self` directly or return a copy.


see what we use elsewhere for this

There isn't great consistency for the description of inplace parameter. The most common and easy to understand I found is If True, do operation inplace and return None. I'll go with that one, and I'll change bool to boolean.

jreback · 2018-10-07T22:27:13Z

pandas/core/generic.py

 Examples
 --------
+ >>> df = pd.DataFrame({"A": [1, 2, 3]})
 >>> df._set_axis_name("foo")


can you show what the current axis name is prior to setting it.

datapythonista · 2018-10-08T04:20:33Z

pandas/core/generic.py


 Examples
 --------
+ >>> df = pd.DataFrame({"A": [1, 2, 3]})


Thanks for updating the docstring @bkjchoi72, looking much better now, and it fixes the doctest, which was the main goal. Just one comment that would make this docstring even better, besides the comments in the previous review.

Do you mind adding a more real-world example? I know it's the original example, not yours, but setting the axis name to foo doesn't give a good idea of that the axis name is.

We use examples in the code with names of animals, and some property like the number of legs. Could you do something like that, and set the axis to animals or the meaningful name for the example?

If you can also change that in DataFrame.rename_axis() too, which has a example with foo as well, that would be great.

Thanks for the suggestion. I'll make those changes.

datapythonista

looking good, just two small things

datapythonista · 2018-10-09T05:50:16Z

pandas/core/generic.py

+ 0 dog 4
+ 1 cat 4
+ 2 monkey 2
+ >>> df.rename_axis("id", axis="columns")


In this case, the concept id refers to name and legs, which does not make sense.

It could make more sense to have the name as the index, and then may be a column num_legs and a column num_arms and call the axis limbs.

Makes sense

datapythonista · 2018-10-09T05:50:46Z

pandas/core/generic.py

- 0 1 4
- 1 2 5
- 2 3 6
+ >>> df = pd.DataFrame({"name": ["dog", "cat", "monkey"], "legs": [4, 4, 2]})


this is longer than 79 characters, isn't it?

datapythonista

The example looks great. Added some more comments, minor things that can be improved, and then should be ready to be merged.

datapythonista · 2018-10-09T10:46:22Z

pandas/core/generic.py

- 1 2 5
- 2 3 6
+ >>> df = pd.DataFrame({"num_legs": [4, 4, 2],
+ ... "num_arms": [0, 0, 2]},


the indentation of this line looks wrong

pandas/core/generic.py

datapythonista · 2018-10-09T10:48:07Z

pandas/core/generic.py

 def _set_axis_name(self, name, axis=0, inplace=False):
 """
- Alter the name or names of the axis.
+ Alter the label(s) of the axis.


This doesn't seem correct. Altering the labels would be changing the names of the columns, not the name of the axis itself.

And small thing, but alter doesn't necessarily need to be correct. If the axis doesn't have a name, it's setting it, but not altering. Set would be better I think.

datapythonista · 2018-10-09T10:48:39Z

pandas/core/generic.py

- 0 or 'index' for the index; 1 or 'columns' for the columns
- inplace : bool
- whether to modify `self` directly or return a copy
+ Labels(s) to set.


Same as before, label is the names of each individual column, not the name of the axis

datapythonista · 2018-10-09T10:50:10Z

pandas/core/generic.py

 Returns
 -------
- renamed : same type as caller or None if inplace=True
+ Series, DataFrame, Panel, or None


Can you remove Panel here. While it's correct, Panel it's already deprecated and will disappear in couple of months, so not worth having it.

datapythonista · 2018-10-09T10:53:32Z

pandas/core/generic.py

+ cat 4
+ monkey 2
+ >>> df.index = pd.MultiIndex.from_product(
+ ... [["mammal"], ['dog', 'cat', 'monkey']])


can you try to present this using a better indentation. I think this is not pep8 compatible

bkjchoi72 · 2018-10-10T03:44:37Z

@jreback @datapythonista Thanks for the patience guys. Let me make adjustments based on your feedback.

…ropriate.

datapythonista

lgtm, thanks @bkjchoi72 for the fixes

datapythonista · 2018-10-10T18:06:31Z

@bkjchoi72 can you take a look at the errors in travis please? they look like genuine errors in your PR.

bkjchoi72 · 2018-10-11T05:13:07Z

@datapythonista The tests are fixed. Thanks for your time and guidance!

datapythonista · 2018-10-11T06:41:16Z

did you run flake8 --doctests pandas/core/generic.py? I think there is a wrong indentation in your example (note that the command will show errors in other parts of the file that you can ignore).

bkjchoi72 · 2018-10-11T06:54:00Z

@datapythonista I don't see indentation errors related to my changes. I modified lines 1120 - 1191 and 1207 -1258. Below is the output of flake8 --doctests pandas/core/generic.py.

$ flake8 --doctests pandas/core/generic.py pandas/core/generic.py:659:13: F821 undefined name 'p' pandas/core/generic.py:660:13: F821 undefined name 'p' pandas/core/generic.py:3195:13: F821 undefined name 'df' pandas/core/generic.py:3200:13: F821 undefined name 'df' pandas/core/generic.py:3205:13: F821 undefined name 'df' pandas/core/generic.py:3211:13: F821 undefined name 'df' pandas/core/generic.py:3218:13: F821 undefined name 'df' pandas/core/generic.py:3222:13: F821 undefined name 'df' pandas/core/generic.py:3227:13: F821 undefined name 'df' pandas/core/generic.py:7545:71: F721 syntax error in doctest pandas/core/generic.py:7548:13: F821 undefined name 's' pandas/core/generic.py:7556:13: F821 undefined name 's' pandas/core/generic.py:7567:13: F821 undefined name 's' pandas/core/generic.py:7599:52: F721 syntax error in doctest pandas/core/generic.py:7603:13: F821 undefined name 'df2'

None in the range that I modified shows up from my end. Do you have specific line numbers that are violating pep8 standards?

datapythonista · 2018-10-11T07:02:19Z

pandas/core/generic.py

- 2 3 6
+ >>> df = pd.DataFrame({"num_legs": [4, 4, 2],
+ ... "num_arms": [0, 0, 2]},
+ ... ["dog", "cat", "monkey"])


this is where I think the ['dog'... should be indented as the {, and not one more. Not sure why is not being captured.

It seems like 1173 and 1174 both need to be aligned with {

doesn't make sense to me, I think it's a flake8 bug, but let's leave it like this is the linting fails with what I think it's the right indentaiton.

Actually, it makes sense to adjust line 1174 only since the argument is composed of two things: dict and list

flake8 still passes with the suggested indentation.

datapythonista · 2018-11-03T07:18:21Z

Sorry for the delay @bkjchoi72, can you fix the conflicts, so we can merge?

datapythonista · 2018-11-09T14:54:35Z

@bkjchoi72, do you have time to fix the conflicts?

bkjchoi72 · 2018-11-10T02:51:28Z

@datapythonista I'll get to it this weekend.

…ocstring

datapythonista · 2018-11-19T21:16:26Z

Rebased, and added some more fixes to the docstring.

@Dr-Irv @jreback can you take a look?

jreback · 2018-11-20T02:21:11Z

thanks @bkjchoi72

Dr-Irv · 2018-11-26T15:36:35Z

@datapythonista I was on vacation for the past 10 days, so just saw this now. All looks good to me.

DOC: Fixed the doctsring for _set_axis_name (GH 22895)

a5fd1c3

bkjchoi72 added 2 commits October 4, 2018 20:14

Added a new line after the first triple quotes

5dbbbd1

Removed the new line after the first triple quotes

bc865d4

Merge branch 'master' into GH22895_fix_docstring_for_set_axis_name

457c58e

gfyoung added the Docs label Oct 6, 2018

gfyoung approved these changes Oct 6, 2018

View reviewed changes

jreback requested changes Oct 7, 2018

View reviewed changes

datapythonista reviewed Oct 8, 2018

View reviewed changes

bkjchoi72 added 4 commits October 8, 2018 04:38

Merge branch 'master' into GH22895_fix_docstring_for_set_axis_name

e469e4e

Merge branch 'master' into GH22895_fix_docstring_for_set_axis_name

7fb8a52

Modified the description of parameters. Changed the example to animals.

067fa8d

Modified the example for DataFrame.rename_axis()

71f3909

datapythonista reviewed Oct 9, 2018

View reviewed changes

Changed the example so that it makes sense

a7eb3d8

datapythonista reviewed Oct 9, 2018

View reviewed changes

bkjchoi72 added 2 commits October 9, 2018 23:53

Fixed the indentation and changed the word "alter" to "set" where app…

96a5dd1

…ropriate.

Merge branch 'master' into GH22895_fix_docstring_for_set_axis_name

22ef507

datapythonista approved these changes Oct 10, 2018

View reviewed changes

bkjchoi72 added 2 commits October 10, 2018 20:45

Fixed failing docstring test

b030a6b

Merge branch 'master' into GH22895_fix_docstring_for_set_axis_name

6d76d44

datapythonista reviewed Oct 11, 2018

View reviewed changes

Adjusted indentation

adc2694

Merging from master and fixing validation issues in the rename_axis d…

9d944e8

…ocstring

jreback added this to the 0.24.0 milestone Nov 20, 2018

jreback approved these changes Nov 20, 2018

View reviewed changes

jreback merged commit 7a6ecf7 into pandas-dev:master Nov 20, 2018

devin-petersohn mentioned this pull request Feb 19, 2019

Remove spurious MultiIndex creation in _set_axis_name #25371

Merged

2 tasks

Uh oh!

Uh oh!

DOC: Fixed the doctsring for _set_axis_name (GH 22895) #22969

DOC: Fixed the doctsring for _set_axis_name (GH 22895) #22969

Uh oh!

Conversation

bkjchoi72 commented Oct 3, 2018 • edited by gfyoung Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

pep8speaks commented Oct 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated on October 09, 2018 at 08:34 Hours UTC

codecov bot commented Oct 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

gfyoung left a comment

Choose a reason for hiding this comment

gfyoung commented Oct 6, 2018

Uh oh!

Choose a reason for hiding this comment

bkjchoi72 Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkjchoi72 commented Oct 10, 2018

datapythonista left a comment

Choose a reason for hiding this comment

datapythonista commented Oct 10, 2018

bkjchoi72 commented Oct 11, 2018

datapythonista commented Oct 11, 2018

bkjchoi72 commented Oct 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista commented Nov 3, 2018

datapythonista commented Nov 9, 2018

bkjchoi72 commented Nov 10, 2018

datapythonista commented Nov 19, 2018

jreback commented Nov 20, 2018

Dr-Irv commented Nov 26, 2018

Labels

6 participants

bkjchoi72 commented Oct 3, 2018 •

edited by gfyoung

Loading

pep8speaks commented Oct 3, 2018 •

edited

Loading

codecov bot commented Oct 5, 2018 •

edited

Loading

bkjchoi72 Oct 9, 2018 •

edited

Loading