-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
[Bug]: v3.5.2 causing plot to crash when plotting object with __array__
method
#22973
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
Comments
OK, sorry about that - its pretty hard to track all the ways we can coerce to numpy, and we are working hard to consolidate them. Can astropy make a mocked up class for us that we can use for testing that should work? Currently looks like we are supporting |
Its pretty easy to add a check for
|
I think |
I am going to ping @keflavich and @mhvk here in case they have insights on |
The private before the public? |
I would say actual protocol rather than ad-hoc protocol. |
@jklymak - |
Thanks, that makes sense, though pandas definitely was failing, so we had to add the unpacking here (I don't know if you saw the diff for #22141 for what we were doing before that PR?) We are completely untested on custom We are always delicate about what point we coerce to numpy because sometimes objects we get passed have units, and we don't want to strip those out too early. Hence the desire to consolidate all this, and not rely on |
For instance, the fix in #22975 fails the unit interface tests.... |
@jklymak - very much appreciated that you try to remove too much explicit casting to numpy arrays! It seems reasonable to leave it to objects that are passed in to try to be good enough duck types. |
Here is a minimal reproducer class based on import numpy as np
import matplotlib.pyplot as plt
class Kernel:
def __init__(self, array):
self._array = np.asanyarray(array)
def __array__(self):
return self._array
@property
def shape(self):
return self._array.shape
kernel = Kernel([1, 2, 3, 4, 5])
plt.plot(kernel) |
@WilliamJamieson Thanks a lot. We can base a test around this, and make sure we don't break you (and others presumably) in the future. And again, sorry for the hassle. |
Just linking with #22645 |
see matplotlib/matplotlib#22973, matplotlib/matplotlib#22879, and matplotlib/matplotlib#22560 It is not clear to me as to which is the standard interface for unit handling (for eg, hist still doesn't handle unit by default)
Bug summary
In astropy/astropy#13209 a potential bug with v3.5.2 has been found wherein a previously working (e.g. in v3.5.1) plot of one of kernel objects now fails. This failure seems to stem from
plot
no-longer coercing the the kernel object correctly. This seems to be related to the changes in #22141. In any case we have an object with a valid__array__
method which was previously plotting correctly, but now produces a crash.Code for reproduction
Actual outcome
Expected outcome
See: https://docs.astropy.org/en/latest/_images/astropy-convolution-Gaussian1DKernel-1.png
Additional information
No response
Operating system
No response
Matplotlib Version
3.5.2
Matplotlib Backend
No response
Python version
No response
Jupyter version
No response
Installation
pip
The text was updated successfully, but these errors were encountered: