BUG np.where half-initializes subclass of output

In numpy 1.9, the behaviour of np.where(condition, first, second) changed, in that the output no longer always is an ndarray instance but an instance of the subclass of first. However, __array_finalize__(self, obj) is called with obj=None, and hence the subclass can not properly initialize itself (according to the documentation [1], None is reserved for the case where a new instance is created via __new__). This change in behaviour has caused some problems (e.g., astropy/astropy#2958, also discussion in astropy/astropy#1274).

I should add that I do not necessarily believe the old behaviour was preferable, but it would be good if the current behaviour is documented.

Going forward, an option which I think would be ideal (though not necessarily practical...), is if np.where would do the equivalent of

out = first.copy()
out[condition] = second[condition]

In this way, subclasses can check if the second argument is consistent with the first (e.g., for our Quantity class, check the units and possibly convert). This might be done most easily if, somehow, np.where could be turned into a ufunc and passed through the __numpy_ufunc__ mechanism.

[1] http://docs.scipy.org/doc/numpy/user/basics.subclassing.html

where now uses the iterator to allocate the output, but when it allocates the result it propagates the subtype but calls the constructor without an object, like a direct __new__ call

hacking it in using the input object with the hightest priority should fix this and the testsuite succeeds.
But I am a skeptical if this is safe for all cases, our testsuite is leaky in regards to these.
Do we need a new iterator flag to call PyArray_NewFromDescr with non-null obj argument from the iterator output allocation?

@mhvk

branch with the changes:
https://github.com/juliantaylor/numpy/tree/iter-subobj

@mhvk can you try if that branch fixes your issues?

on second though where is currently completely subclass unsafe as there is no way to preserve extra information like masks.
should we revert back to 1.8 dropping of subclasses? or change it to use indexing so __getitem__ __setitem__ are used as suggested?

@juliantaylor

@juliantaylor - I agree that it would remain unsafe for subclasses. I wondered whether my suggestion was much slower, but it turns out it is not too bad, at least for large arrays:

import numpy as np
def my_where(condition, first, second):
    condition, first, second = np.broadcast_arrays(condition, first, second)
    condition = ~condition
    result = first.copy()
    result[condition] = second[condition]
    return result

a = np.random.uniform(size=100000)
condition = a > 0.5
%timeit np.where(condition, a, 0.)
# 100 loops, best of 3: 2.07 ms per loop
%timeit my_where(condition, a, 0.)
# 100 loops, best of 3: 2.36 ms per loop

For smaller arrays, it is worse. E.g., for size=100, one becomes dominated by all the initialisation: 13.5 and 96.7 micro-sec, respectively.

Note that implementing where() via indexing requires a special case for 0d
inputs.

where(True, 5, 10)

works, but

result[condition] = ...

will fail because of how 0d indexing works.

where can be double as fast as indexing if the condition is more friendly to branch prediction (dense or sparse) but that should not be an issue, we can add a wrapper python function that does indexing on subclasses and calls to the existing C function for ndarrays
for 1.9.1 we should probably just disable the broken subclass keeping as it brakes code.

result[np.asarray(condition)] is a workaround until our deprecation is all done and we can change it in master.

@juliantaylor

@juliantaylor - I saw in #5105 that you hope to produce a version that handles subclasses correctly. Let me know if I can help in any way. Also, shall I raise a new issue reminding us of this?

too bad broadcast_arrays doesn't preserve subtypes, else this would probably already be it:

        a = np.asanyarray(a)
        b = np.asanyarray(b)
        cond, a, b = np.broadcast_arrays(cond, a, b)
        res = b.copy()
        res[cond] = a[cond]
        return res

I wonder if thats fixable

@seberg

See #4622! (about to be merged by @seberg)

p.s. This is indeed why I thought of my example. Note that broadcast_arrays does upgrade to an array, so I think one could even omit the asanyarray.

@mhvk

np.where works like a ufunc, though I know it isn't implemented that way internally. So I think the right solution to this is probably to simply use __numpy_ufunc__ and let subclasses/duck-types override the functionality that way.

@mhvk's proposed solution is interesting, but it won't work for all duck-array types:

out = first.copy()
out[condition] = second[condition]

The problem is that some of the duck-array types for which where is most useful are actually immutable (e.g., dask.array). These types need their own solution, which they can implement with __numpy_ufunc__.

I should also point out that my code example used to work because the condition was a masked array. Depending on the type of first would have been misleading in my case as my example has just an integer there.

...and looking over my company's internal bug tracker, this may be the source of another bug that was customer-facing, causing our shipped netCDF files to have very negative values in places that should have been masked, and our contours to look a bit silly. We never figured out the exact cause of the issue, but I am now going to see if an np.where() was used.

@juliantaylor

Perhaps we should raise an error until the subclass issue is fixed. @juliantaylor Have you had any time to pursue your solution?

@juliantaylor

I think I would be more comfortable with a warning. The sneaky nature of
this problem indicates that the results are more like a degraded result
rather than "wrong", especially in the face of duck-typing elsewhere. A
warning would have pulled me to this spot in the code faster, and I would
have reported it sooner as such. It is quite possible that other people
have encountered similar problems and merely shrugged and worked around it,
or didn't figure out the source of it.

On Wed, Jul 29, 2015 at 4:00 PM, Charles Harris [email protected]
wrote:

Perhaps we should raise an error until the subclass issue is fixed.
@juliantaylor https://github.com/juliantaylor Have you had any time to
pursue your solution?

—
Reply to this email directly or view it on GitHub
#5095 (comment).

So this is still up in the air. Pushing off to 1.12.

Closing since one can now override np.where with __array_function__. Hopefully, eventually where becomes a proper ufunc.

Navigation Menu

Repository navigation

BUG np.where half-initializes subclass of output #5095

7 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

BUG np.where half-initializes subclass of output #5095

Description

Activity

juliantaylor commented on Sep 22, 2014

juliantaylor commented on Sep 22, 2014

juliantaylor commented on Sep 22, 2014

mhvk commented on Sep 22, 2014

njsmith commented on Sep 22, 2014

juliantaylor commented on Sep 23, 2014

seberg commented on Sep 23, 2014

mhvk commented on Sep 23, 2014

juliantaylor commented on Sep 23, 2014

mhvk commented on Sep 23, 2014

mhvk commented on Sep 23, 2014

7 remaining items

shoyer commented on Jul 29, 2015

WeatherGod commented on Jul 29, 2015

WeatherGod commented on Jul 29, 2015

charris commented on Jul 29, 2015

WeatherGod commented on Jul 29, 2015

charris commented on Jan 21, 2016

mhvk commented on Jun 17, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions