Fix comparisons of parameters with arrays #3912
Conversation
…s with scalars or other arrays) to return a Numpy bool array in a Numpy-like fashion. Also wraps the parameter value and the value it is compared with using np.asanyarray so that Numpy subclasses can be returned (in anticipation of Quantity support).
| @@ -728,22 +728,22 @@ def __rtruediv__(self, val): | |||
| return val / self.value | |||
|
|
|||
| def __eq__(self, val): | |||
| return (np.asarray(self) == np.asarray(val)).all() | |||
| return np.asanyarray(self) == np.asanyarray(val) | |||
There was a problem hiding this comment.
Note that asanyarray will just call the __array__ method and thus return a plain array! You need np.asanyarray in __array__ for this to work.
With that, I think it this should be something like
def __eq__(self, val):
return self.__array__().__eq__(val)
This would let ndarray.__eq__ take care of interpreting val (no need to do it yourself).
The above assumes you want Parameter to always look array-like (probably good). But maybe _value is already like that? If so, then maybe even cleaner would be:
if self._model:
return self.value.__eq__(val)
else:
return False
For the comparisons, you are allowed to raise an error, so it would just be
def __lt__(self, val):
return self.value.__lt__(val)
There was a problem hiding this comment.
Note that asanyarray will just call the array method and thus return a plain array!
That was the intent, I think. It's not supposed to return a Parameter object or anything like that (though I will want it to return a Quantity for parameters with units). Currently in #3852 the .value attribute just returns the raw value, and not the Quantity. So I still need to use np.asanyarray. However, I did not fix things yet so that __array__ can return a Quantity. Wouldn't that be good enough?
There was a problem hiding this comment.
Yes, if __array__ can also return a Quantity, the first suggestion I gave will work fine.
There was a problem hiding this comment.
Though there's no real reason to call methods like __eq__ explicitly either. I could just write self.__array__() == other.
There was a problem hiding this comment.
I think you are right that is fine in this case; in principle, the self.__array__().__eq__(val) could return NotImplemented, which would then mean val would have a chance to try (which might recognize Parameter and want to do something else than compare with value. But this is probably too far-fetched!
…astropy/pull/3912/files#r33819840 . Also fix the misbehavior in Parameter.__array__ which returned an empty array for unbound Parameters. Unbound parameters really just shouldn't be convertible to arrays at all. This change necessitated a special case in Parameter.__eq__ so that unbound parameters can be reasonably compared to other objects (the previous behavior was still using Parameter.__array__ which made not sense, since comparing unbound parameters to objects was equivalent to comparing that object to an empty array).
Fix comparisons of parameters with arrays
(or of array-like parameters with scalars or other arrays) to return a Numpy bool array in a Numpy-like fashion. Also wraps the parameter value and the value it is compared with using np.asanyarray so that Numpy subclasses can be returned (in anticipation of Quantity support).
This addresses a comment from @mhvk made here.
This is unfortunately a backward-incompatible change, with no straightforward way to deprecate the old behavior. Some could argue, however, that the old behavior was a bug anyways (or at least ill-conceived) and probably isn't relied on anywhere except with gritting of teeth.
When this is merged I will rebase #3852 to incorporate it as well.