Skip to content

DOC: Ensure that we add documentation also as to the dict for types #18775

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.

Sign up for GitHub

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

Jump to bottom
Merged
WarrenWeckesser merged 2 commits into from
Oct 6, 2021
Merged

DOC: Ensure that we add documentation also as to the dict for types #18775

WarrenWeckesser merged 2 commits into from
Oct 6, 2021

Conversation

@seberg
Copy link
Member

@seberg seberg commented Apr 14, 2021

This ensures that help(np.dtype) produces a result. I am not
exactly sure why it picks up __doc__ from the dict instead of
tp_doc right now. It probably is due to the combination of
inheritance and the fact that the dict always includes None
and gets preference during inheritance.
(That probably makes a lot of sense to not inherit the type
docstring by default.)

Modifying the dictionary directly is not really good style, either,
but hopefully works.

Closes gh-18740

@seberg
Copy link
Member Author

seberg commented Apr 14, 2021 •
edited
Loading

OK, this doesn't quite seem to work, because it overshadows the getset of the instance? Hmmm...

EDIT: Well, maybe the solution is to add a special case just for np.dtype right now...

@eric-wieser
Copy link
Member

eric-wieser commented Apr 15, 2021

Are metaclasses related at all here, or is this just a heap type problem?

@seberg
Copy link
Member Author

seberg commented Apr 15, 2021

@eric-wieser yeah, that would make sense, but I am not setting the HEAPTYPE flag on np.dtype right now, which is why I am confused. So my guess was that inheritance causes the __dict__ value to be used instead of the tp_doc one (although I am not quite sure where that happens in pythons typeobject.c

@charris
Copy link
Member

charris commented Apr 21, 2021

close/reopen

@charris charris closed this Apr 21, 2021
@charris charris reopened this Apr 21, 2021
@charris charris added 25 - WIP 09 - Backport-Candidate PRs tagged should be backported labels Apr 30, 2021
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label May 17, 2021
@charris charris mentioned this pull request Jul 2, 2021
Open
@seberg
DOC: Ensure that we add documentation also as to the dict for types
9975cc8 
This ensures that `help(np.dtype)` produces a result.  I am not
exactly sure why it picks up `__doc__` from the dict instead of
`tp_doc` right now. It probably is due to the combination of
inheritance and the fact that the dict always includes `None`
and gets preference during inheritance.
(That probably makes a lot of sense to not inherit the `type`
docstring by default.)

Modifying the dictionary directly is not really good style, either,
but hopefully works.

Closes numpygh-18740
@seberg seberg force-pushed the dtype-doc-missing branch from abf8ffd to 9975cc8 Compare October 4, 2021 18:50
@seberg
Copy link
Member Author

seberg commented Oct 4, 2021

After chasing around how ufuncs add docstrings (we have a function to do it via Python, which claims it is used but is not actually used)...

The only thing that was necessary here was to only put the doc-string into the dict only if the dict contains None for __doc__ (which means PyType_Read() probably filled it in automatically.

This actually means/fixes that np.float16(3.).__doc__ also correctly shows the np.float16.__doc__ now.

@seberg seberg mentioned this pull request Oct 4, 2021
Closed
@seberg seberg added 09 - Backport-Candidate PRs tagged should be backported and removed 25 - WIP labels Oct 4, 2021
WarrenWeckesser
WarrenWeckesser approved these changes Oct 5, 2021
View reviewed changes
Copy link
Member

@WarrenWeckesser WarrenWeckesser left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @seberg. I noticed one small typo.

WarrenWeckesser
WarrenWeckesser reviewed Oct 5, 2021
View reviewed changes
@seberg @WarrenWeckesser
Apply suggestions from code review
7990348 
Co-authored-by: Warren Weckesser <[email protected]>
@WarrenWeckesser WarrenWeckesser merged commit fb2509c into numpy:main Oct 6, 2021
@WarrenWeckesser
Copy link
Member

WarrenWeckesser commented Oct 6, 2021

Thanks @seberg.

@seberg seberg deleted the dtype-doc-missing branch October 6, 2021 00:40
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Oct 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WarrenWeckesser WarrenWeckesser WarrenWeckesser approved these changes

Assignees

No one assigned

Labels

04 - Documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

dtype missing docstrings since 1.20.1

4 participants

@seberg @eric-wieser @charris @WarrenWeckesser