Skip to content

DOC: Add documentation to __add__ #50527

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 11 commits into from
Jan 30, 2023

Conversation

jessica-writes-code
Copy link
Contributor

I'm not sure what the convention on documenting (or not documenting) dunder methods is. If there's a better place to add documentation to resolve #44784 , happy to do that!

@datapythonista datapythonista added Docs Numeric Operations Arithmetic, Comparison, and Logical operations labels Jan 4, 2023
Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

Thanks @jessica-writes-code, nice work.

Just a comment about the examples. Also, is this docstring being rendered anywhere in the docs? I don't remember having a section for the operators.

@datapythonista datapythonista changed the title Add documentation to __add__ DOC: Add documentation to __add__ Jan 4, 2023
@jessica-writes-code
Copy link
Contributor Author

Thanks for your review @datapythonista !

Pushed a commit to update the example using animal names. Thanks for pointing that out!

To your initial question: I am not aware of any location where this docstring would get rendered the docs, because -- exactly to your point -- there doesn't seem to be a section for operators. Happy to either (a) add such a section (or subsection, etc.) to the docs or (b) relocate this documentation elsewhere, if either of those are desireable.

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

Thanks @jessica-writes-code, just couple of minor thing.

I guess we can add __add__ and the other operators to the documentation. I see that for add and the other we seem to be reusing the same docstring for all them. No strong preference on what to do here. Regarding whether to add the magic methods, and regarding what docstrings to reuse. @MarcoGorelli what do you think?

@jessica-writes-code
Copy link
Contributor Author

Excellent. Thank you @datapythonista ! Pushed a few more commits to address your comments and added __add__ to the docs. I'm going to start work on a reusable doc string for the other magic methods, pending comment from @MarcoGorelli .

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Hey @jessica-writes-code

First, thanks for working on this. Second, sorry for the delay, on this now.

This looks good to me - I'm not overly keen on sharing docstrings, this looks fine to me

Just got a comment on the indendation of the examples - I think if you just run it locally and copy-and-paste from the result then it should look fine

Comment on lines 166 to 168
elk height moose weight
elk NaN NaN NaN NaN
moose NaN NaN NaN NaN
Copy link
Member

@MarcoGorelli MarcoGorelli Jan 22, 2023

Choose a reason for hiding this comment

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

this still doesn't look quite aligned, running this locally I see

       elk  height  moose  weight
elk    NaN     NaN    NaN     NaN
moose  NaN     NaN    NaN     NaN

does it work to just copy-and-paste that, and then add 8 spaces to each line? If you use tab then it may not work exactly

(likewise for the others)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, that doesn't seem to work. The most recent commit contains the results copy/pasted from running locally, with 8 spaces added to the left. That seems to mess up header alignment. (Adding another 4 spaces to the header is a slight improvement, but doesn't seem to 100% fix the issue.)

Copy link
Contributor Author

@jessica-writes-code jessica-writes-code Jan 22, 2023

Choose a reason for hiding this comment

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

I stand corrected and am apparently just really bad at spacing. Most recent commit (hopefully) has correct alignment. Apologies for all the back-and-forth!

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @jessica-writes-code !

Over to @datapythonista for any further thoughts

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

lgtm, just a minor comment.

I published the rendered docs here, in case you want to have a look: https://pandas.pydata.org/preview/50527/docs/reference/api/pandas.DataFrame.__add__.html

@MarcoGorelli MarcoGorelli added this to the 2.0 milestone Jan 30, 2023
@MarcoGorelli
Copy link
Member

good one - thanks @jessica-writes-code !

@MarcoGorelli MarcoGorelli merged commit 77e97d2 into pandas-dev:main Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Numeric Operations Arithmetic, Comparison, and Logical operations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: Broadcasting for binary operators
3 participants