8000 ENH: add __dict__ to the ufunc object · Issue #26233 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

ENH: add __dict__ to the ufunc object #26233

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
Closed
lpsinger opened this issue Apr 8, 2024 · 15 comments · Fixed by #27735
Closed

ENH: add __dict__ to the ufunc object #26233

lpsinger opened this issue Apr 8, 2024 · 15 comments · Fixed by #27735
Labels
01 - Enhancement Numpy 2.0 API Changes Project Possible project, may require specific skills and long commitment
Milestone
2.2.0 release

Comments

@lpsinger
Copy link
Contributor
lpsinger commented Apr 8, 2024

Proposed new feature or change:

I maintain several Python packages with C extensions that define ufuncs and generalized ufuncs. I have been relying upon the add_newdoc_ufunc function to add docstrings to those functions because it is such a pain to write legible multiline strings in C. According to the Numpy 2.0.0 migration guide, add_newdoc_ufunc is "an internal function and doesn’t have a replacement." Is it possible to add it back to the public namespace as numpy.lib.add_newdoc_ufunc?
@charris charris added this to the 2.0.0 release milestone Apr 8, 2024
@mattip
Copy link
Member
mattip commented Apr 9, 2024

This function messes with the PyTypeObject's tp_doc attribute after PyType_Ready is called, which is problematic. See #10167. To create multiline docstrings, couldn't you add a \n to the end of the C string?

@seberg
Copy link
Member
seberg commented Apr 9, 2024

Not sure how much you need it, I am tempted to say: just use the private version. We will keep pretending that we can just delete it at some point (i.e. doing this break in some future), but I don't expect that future to happen very soon.

@lpsinger
Copy link
Contributor Author
lpsinger commented Apr 9, 2024

What about a patch to add a setter for the doc property?

@seberg
Copy link
Member
seberg commented Apr 9, 2024 •
edited
Loading

@lpsinger if that works, that seems like a good idea to me. UFuncs currently do not have a __dict__ so unless we want to change that, we need to add the properties explicitly (which we can just do, though).
We should actually probably do the same for __module__ and maybe also __qualname__ for pickling and inspection (who knows, maybe also __signature__... maybe we should just add a __dict__!? Python is a language that generally is OK with allowing monkey patching after all).

The one caveat is that it might be better to not modify the size of the ufunc object in bug-fix releases (we may actually have some empty slots, though).

EDIT: Sorry for double post, github was misbehaving.

@WarrenWeckesser
Copy link
Member

... maybe we should just add a __dict__!?

FWIW, this is a feature I would like, but it was never a high enough priority to investigate an implementation. The use-case is a function where an integer parameter is effectively an enum, and I wanted to store the allowed values as attributes on the ufunc. See the fifth bullet of the list here: https://github.com/WarrenWeckesser/numpy-notes/blob/main/enhancements/gufunc-wishlist.md

@lpsinger
Copy link
Contributor Author
lpsinger commented Apr 9, 2024

Hmm, unfortunately ufunc_get_doc does not return ufunc->doc; it does some formatting to add the signature in order to compute the value to return for __doc__. So simply adding a setter to go with it would not make a lot of sense, because

>>> func.__doc__ = 'foo'
>>> assert func.__doc__ == 'foo'

would fail.

What about adding a ufunc.doc prop?

lpsinger added a commit to lpsinger/ligo.skymap that referenced this issue Apr 9, 2024
@lpsinger
Update import of add_newdoc_ufunc for Numpy 2.0.0
b4b790a 
The funcion `add_newdoc_ufunc` is no longer part of the Numpy
public API, although according to upstream developers it is not
going away any time soon.

See numpy/numpy#26233.
@seberg
Copy link
Member
seberg commented Apr 10, 2024

So simply adding a setter to go with it would not make a lot of sense

Maybe a setter can work, though? You could give it a logic of:

@property
def __doc__(self):
    if self._doc_obj:
        return self._doc_obj

    # old logic.

@__doc__.setter
def __doc__(self, value):
    self._doc_obj = value
    # Or if need be, leak it into the doc string but store the info
    # that __doc__ was called in some way.

If we add the __dict__, I don't mind if we store it as a _ufunc_doc in the dict also (doc lookup doesn't need to be optimized).

@lpsinger
Copy link
Contributor Author

So simply adding a setter to go with it would not make a lot of sense

Maybe a setter can work, though? You could give it a logic of:

The problem is that the getter for __doc__ must return the concatenation of the computed signature text plus the library-provided ufunc->doc string, whereas the setter would only take the latter. We certainly could add a setter, but as I said it would not obey the following behavior of most properties:

>>> obj.prop = value
>>> assert obj.prop == value

Are you OK with that?

@seberg
Copy link
Member
seberg commented Apr 11, 2024

Ah, you are right. I thought we would ask you to include the signature in your doc string, but that isn't great.
There may be a simple solution, but I haven't tried around with it: We could try to define the __text_signature__ instead and always exclude the signature from the docs.
That seems very right, but not sure if there is some subtlety that makes this harder.

(There was some discussion about using __text_signature__ earlier, I think. @eric-wieser would you know if there is some caveat, or whether this was just never done.)

@rgommers
Copy link
Member

I haven't followed in detail, but just a quick note that there is recent discussion/work on __text_signature__ in CPython, see https://discuss.python.org/t/type-signatures-for-extension-modules-pep-draft/43914

@seberg
Copy link
Member
seberg commented Apr 15, 2024

Just to come back: if assignment to __doc__ is weird, I would say a _set_doc or so method should OK, with the understanding that we might replace it with a better way at some point...

@mattip mattip removed this from the 2.0.0 release milestone Jun 12, 2024
@ngoldbaum
Copy link
Member

For now we're going to say that you should just go ahead and use the private version if you really need it and subscribe to this issue. We will add a __dict__ to ufunc objects in the future, allowing us to deprecate the internal function.

So, you'll be able to replace np.add_newdoc_ufunc with np._core.umath._add_newdoc_ufunc and avoid the deprecation warning.

Sorry for the trouble, this corner case was missed during the API transition.

@ngoldbaum ngoldbaum added this to the 2.1.0 release milestone Jun 12, 2024
@ngoldbaum ngoldbaum changed the title ENH: Restore numpy.lib.add_newdoc_ufunc ENH: add __dict__ to the ufunc object Jun 12, 2024
@charris charris modified the milestones: 2.1.0 release, 2.2.0 release Aug 1, 2024
@mattip
Copy link
Member
mattip commented Aug 7, 2024

We will add a __dict__ to ufunc objects in the future

This might be a nice starter project for someone interested in digging into the C-API. We discussed a triage meeting and decided to leave the milestone. By fixing this people would not need to use internals.

@mattip
Copy link
Member
mattip commented Aug 21, 2024

While this probably shouldn't be on the milestone, it would be a nice enhancement to have so it is nice to be reminded that we should just do this.

@seberg seberg added the Project Possible project, may require specific skills and long commitment label Aug 21, 2024
@ngoldbaum ngoldbaum mentioned this issue Sep 12, 2024
Closed
@ngoldbaum
Copy link
Member

I opened #27383 that allows setting docstrings for ufuncs from Python.

@mtsokol mtsokol mentioned this issue Nov 11, 2024
Merged
@ngoldbaum ngoldbaum mentioned this issue Nov 13, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
01 - Enhancement Numpy 2.0 API Changes Project Possible project, may require specific skills and long commitment
Projects
None yet
Milestone
2.2.0 release
7 participants
@seberg @charris @rgommers @WarrenWeckesser @lpsinger @mattip @ngoldbaum
0