I think it needs a test, or we can break again. Unless you think the Quantities behaviour is a bug, in which case we should bounce the issue back to them.

Well, the "bug" is that np.zeros_like returns a copy of the same object and then fills it with zeros, rather than zeros that creates a new nparray object of the same size (unless the like-parameter is passed, when the type can be set to something else) and then fills it with zero.

This was not clear from the documentation. So any use of np.*_like should be avoided in Matplotlib for things where there may be Quantity involved (and possible other classes). There are 88 cases in the current code base, although maybe half are in tests and examples and so on.

Not sure how to modify our Quantity class to get this behavior though. I've spent some time trying to figure out why it is different, but it seems to boil down to how numpy decides to copy the array.

Here is a simple example to show the problem:

from matplotlib.tests.test_units import Quantity
import astropy.units as u
import numpy as np
mplq = Quantity([1, 2, 3], 'm')
q = [1, 2, 3]*u.m
print(repr(np.zeros_like(mplq, dtype=bool)))
print(repr(np.zeros_like(q, dtype=bool)))

So we would like our Quantity to still return a Quantity object after np.zeros_like for the test to make sense. Right now, testing with our Quantity will work even with the earlier code.

So any use of np.*_like should be avoided

Let me rephrase that: any use of np.*_like combined with setting dtype should be avoided.

There is only one use of that, apart from this, namely in image.py. This may (or may not) be the limiting factor of having units in images, but it is not necessarily and issue, only combined with e.g. np.less where values are set.

OK, this is fine. However, my complaint stands that this should be dealt with earlier than this in the processing of the inputs so that we don't keep having to squash issues like this.

I dug a bit more into it and astropy Quantity inherits from np.ndarray. Doing that to our Quantity mock leads to a crash in the code example above with what looks like an infinite recursion.

Redefining __deepcopy__ does not help either, although the hint in astropy may indicate it is involved:
https://github.com/astropy/astropy/blob/f0e2129aad460ea0db00730f10f98fe799aefb2f/astropy/units/quantity.py#L778-L781

I do not disagree that units should be handled differently. Just that I have very limited insight into it.

It would indeed make sense to get our mock Quantity to behave the same for this case to make the transition to another approach easier.

I am going to cc @mhvk here too in case he has insights. Thanks!

I'll only add that the Quantity class as it exists right now has been crafted to make sure that it works like pint.Quantity (and thus tests that).

Instead of changing it, I'd advise making a different mock for astropy, and possibly make the tests parameterized around the mocks.

Thanks for the info @dopplershift !

I can confirm that pint.Quantity does not have this issue.

import numpy as np
import pint
q = pint.Quantity([1, 2, 3], 'm')
print(repr(np.zeros_like(q, dtype=bool)))

shows that the results of np.zeros_like is an np.array.

So something is slightly different for astropy.Quantity.

Quantity/pint.Quantity is intentionally not an ndarray subclass due to the challenges of getting the subclassing right, and more importantly the complete inability to combine ndarray subclasses.

Pretty sure by design zeros_like (and friends) preserves the type for any subclass it is given.

Pretty sure by design zeros_like (and friends) preserves the type for any subclass it is given.

I agree. Thought it was a convenience method for getting the same shape, not subclass, hence the introduced problem.

I'll try and make a second Quantity mock that subclasses ndarray and see if that has the problematic behavior.

Nah, it is possible to subclass (by getting rid of _getattr_), but to get it to properly fail while assigning values that does not have units, one would need to also introduce a mock unit class.

I, again, suggest that we should instead extend the weekly upcoming dependency tests
with more optional upstream and downstream libraries.

So I think that this is good to go as is.

Maybe pint is light weight enough to add as a test requirement for unit tests.

pint is 300 k, so should not be a problem. astropy (which causes the issue here) is 6-7 MB, so probably not worth it.

According to the above, pint and astropy are basically the same wrt the Quantity class?

pint and astropy are basically the same

AFAIK they are unrelated? astropy.units came from pynbody.

Oh I'm sorry I misunderstood @dopplershift OTOH we should check the blame.

According to the above, pint and astropy are basically the same wrt the Quantity class?

If it is my statements you are referring to, I must have been unclear. However, I think I now have a much briefer summary:

• pint.Quantity behaves as mpl.Quantity when it comes to np.*_like and works with current main.
• astropy.Quantity does not behave as mpl.Quantity when it comes to np.*_like and does not work with current main. I think this is related to astropy.Quantity inheriting from np.ndarray, so the result of np.*_like is the subclass, not an np.ndarray.

Indeed, astropy's Quantity has an __array_function__ override that ensures np.zeros_like produces a Quantity; in principle, this should be standard behaviour also for non-subclasses that aim to mimic an array (e.g., np.zeros_like(dask_array) returns a dask_array). For creating a mask with the same shape, one would probably be better off doing np.zeros(q.shape, dtype=bool).

The job that was failing with matplotlib dev now passes over at astropy. Thank you!