Skip to content

MNT: fix __array__ to numpy #22975

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

Merged
merged 1 commit into from
May 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/matplotlib/cbook/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1333,7 +1333,12 @@ def _check_1d(x):
"""Convert scalars to 1D arrays; pass-through arrays as is."""
# Unpack in case of e.g. Pandas or xarray object
x = _unpack_to_numpy(x)
if not hasattr(x, 'shape') or len(x.shape) < 1:
# plot requires `shape` and `ndim`. If passed an
# object that doesn't provide them, then force to numpy array.
# Note this will strip unit information.
if (not hasattr(x, 'shape') or
not hasattr(x, 'ndim') or
Copy link
Member Author

Choose a reason for hiding this comment

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

This seems to fix the problem. and see the test. However I am not sure if this is really on us, or the Kernel, which is missing ndim. Or maybe we should not be relying on ndim.

Copy link
Member

Choose a reason for hiding this comment

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

Is ndim always len(shape)? (I guess not, but otherwise it make make sense to just use that instead?)

Copy link
Member

Choose a reason for hiding this comment

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

There are 120 ndim in the Python code base (although some are in tests or non-Python-code), so even though we probably should fix it here, in the long run it may be better that Kernel gets ndim.

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is that Kernel does not really properly behave like an ndarray, so it does need to be converted to an array here. We're (ab)using the fact that it does not define ndim as a proxy for "I'm not a proper duck type, just happen to have shape". Really one would like the np.asduckarray that has been discussed in numpy...

len(x.shape) < 1):
return np.atleast_1d(x)
else:
return x
Expand Down
19 changes: 19 additions & 0 deletions lib/matplotlib/tests/test_units.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,3 +264,22 @@ def test_empty_default_limits(quantity_converter):
fig.draw_without_rendering()
assert ax.get_ylim() == (0, 100)
assert ax.get_xlim() == (28.5, 31.5)


# test array-like objects...
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


def test_plot_kernel():
# just a smoketest that fail
kernel = Kernel([1, 2, 3, 4, 5])
plt.plot(kernel)