-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
ENH: Add a __dict__
to ufunc objects and allow overriding __doc__
#27735
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
Conversation
… `__doc__` Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
d6bafc6
to
3eafa3f
Compare
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.
Apart from the nitpick on the tests, this looks great!
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.
FWIW, looks fine to me as well, but also some comments. I mostly still think that we should also note other things like module and qualname (which have very clear use-cases as well).
ccdeffb
to
1aae07d
Compare
1aae07d
to
f853428
Compare
As a followup we should probably set |
Maybe we can sneak in a deprecation warning into |
f853428
to
43080cc
Compare
43080cc
to
60fb218
Compare
343c87a
to
aa8f033
Compare
aa8f033
to
a507bf6
Compare
@ngoldbaum Done! I think all is ready now. |
``__qualname__``. | ||
|
||
* ``_add_newdoc_ufunc`` is now deprecated. ``ufunc.__doc__ = newdoc`` should | ||
be used instead. |
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.
Nit, but it should be split into two files (different categories).
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.
Ah right, done!
We discussed this at the triage meeting and would like to see it merged in. |
I gave the C code another once-over and I think this is correct. I'll merge now. We should definitely update the way we set docstrings internally to make use of this as a followup, fixing #10167 too. |
Hi @ngoldbaum, @mhvk,
This PR fixes #26233 and replaces #27383 (with existing review comments applied).