Skip to content

Attributes on the objects dispatched via _ArrayFunctionDispatcher #24019

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

Open
adrinjalali opened this issue Jun 22, 2023 · 8 comments
Open

Attributes on the objects dispatched via _ArrayFunctionDispatcher #24019

adrinjalali opened this issue Jun 22, 2023 · 8 comments

Comments

@adrinjalali
Copy link
Contributor

adrinjalali commented Jun 22, 2023

Steps to reproduce:

In numpy>=1.25, this is the behavior:

>>> import numpy as np
>>> type(np.mean)
<class 'numpy._ArrayFunctionDispatcher'>
>>> import inspect
>>> inspect.isfunction(np.mean)
False
>>> inspect.ismethod(np.mean)
False
>>> from numpy import _ArrayFunctionDispatcher
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: cannot import name '_ArrayFunctionDispatcher' from 'numpy' (/home/adrin/miniforge3/envs/skops/lib/python3.11/site-packages/numpy/__init__.py)

I would think inspect.isfunction(np.mean) should return True, maybe?

Also, it's odd that the type(np.mean) returns something which cannot be imported, and the actual import is from numpy.core._multiarray_umath import _ArrayFunctionDispatcher.

Ended up observing this since our dispatch mechanism in skops which was dispatching ufunc was ignored since these are no more ufuncs (I think they were before?)

@seberg
Copy link
Member

seberg commented Jun 22, 2023

It quacks enough to pass inspect.isroutine, which is the best that is possible as far as I can tell, see also gh-23307. Not sure what you are doing, but maybe it also makes sense to use type(np.mean).
This was a Python function type, and isn't anymore, there is nothing more to the change (except of course a huge speedup in dispatching) really everything else drops out from that, whether surprising or not.

@adrinjalali
Copy link
Contributor Author

Well, we're dispatching based on the type of objects, in this case np.mean, and now this type is a private thing, which indicates we shouldn't be using it. But I think it's reasonable to expect that the type of something like np.mean to be a public kinda thing.

@seberg
Copy link
Member

seberg commented Jun 22, 2023

Ah well, I don't mind adding it somewhere, but also don't know a particularly good place that is clearly "public".

@adrinjalali
Copy link
Contributor Author

numpy.core.ArrayFunctionDispatcher?

@rgommers
Copy link
Member

I think this type should not be exposed. We should be free to make this kind of change to what is internal infrastructure, so using inspect.isroutine seems much more reasonable than exposing this type.

@adrinjalali
Copy link
Contributor Author

@rgommers this type is exposed via type(np.mean), and it's exposed the wrong way. I really do think the type of something like np.mean should be public, but even if you decide not to make it public, one should be able to import whatever the output of type(obj) is, and in this case, numpy._ArrayFunctionDispatcher cannot be imported, but is reported to be the type of np.mean. We should at least fix this inconsistency.

@rgommers
Copy link
Member

With current main:

>>> type(np.mean)
<class 'numpy._ArrayFunctionDispatcher'>
>>> from numpy._core.overrides import _ArrayFunctionDispatcher
>>> type(np.mean) is _ArrayFunctionDispatcher
True
>>> from numpy._core._multiarray_umath import _ArrayFunctionDispatcher
>>> type(np.mean) is _ArrayFunctionDispatcher
True

The __module__ attribute of np.mean is explicitly set to 'numpy' inside the array_function_dispatch decorator. That goes through several layers of decorators and functools.wraps. I don't really know how to unravel that without spending more time than I have for this. If there's an easy way to make type(np.mean) return what you want here, changing that seems okay. I'd say it's very low-prio for us though, since no one really should make use of anything that is private.

this type is exposed via type(np.mean), and it's exposed the wrong way

I would not call that "exposed". Using type like that is very non-idiomatic. inspect.isroutine should do all you need here, there's nothing you can/should do with a private type in Python.

@seberg
Copy link
Member

seberg commented Oct 20, 2023

The module is hard-coded in the C type structure, very simple to change for someone who wants to.

jvesely added a commit to jvesely/PsyNeuLink that referenced this issue Feb 9, 2024
Numpy functions like np.sum are no longer of type FunctionType since
Numpy moved dispatcher implementation to C in 1.25 [0,1]

Add "type(np.sum)" in addition to types.FunctionType to  use the same
path for Nnumpy functions in numpy 1.25.

[0] numpy/numpy@60a858a
[1] numpy/numpy#24019

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
jvesely added a commit to jvesely/PsyNeuLink that referenced this issue Feb 9, 2024
Numpy functions like np.sum are no longer of type FunctionType since
Numpy moved dispatcher implementation to C in 1.25 [0,1]

Add "type(np.sum)" in addition to types.FunctionType to  use the same
path for Nnumpy functions in numpy 1.25.

[0] numpy/numpy@60a858a
[1] numpy/numpy#24019

Closes: PrincetonUniversity#2908
Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
jvesely added a commit to PrincetonUniversity/PsyNeuLink that referenced this issue Feb 10, 2024
Numpy functions like np.sum are no longer of type FunctionType since
Numpy moved dispatcher implementation to C in 1.25 [0,1]

Add "type(np.sum)" in addition to types.FunctionType to  use the same
path for Numpy functions in numpy 1.25.

[0] numpy/numpy@60a858a
[1] numpy/numpy#24019

Closes: #2908

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants