8000 NumPy ignores __array_priority__ when the right-hand-side operand is a subclass of ndarray · Issue #4766 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

NumPy ignores __array_priority__ when the right-hand-side operand is a subclass of ndarray #4766

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.

Sign up for GitHub

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

Jump to bottom
Closed
madsbk opened this issue Jun 2, 2014 · 5 comments
Closed

NumPy ignores __array_priority__ when the right-hand-side operand is a subclass of ndarray #4766

madsbk opened this issue Jun 2, 2014 · 5 comments

Comments

@madsbk
Copy link
Contributor
madsbk commented Jun 2, 2014

The following code does not output "sub_class radd" as expected since NumPy ignores __array_priority__ when the right-hand-side operand is a subclass of ndarray.

import numpy as np
class sub_class(np.ndarray):
    __array_priority__ = 1000
    def __add__(self, rhs):
        print "sub_class add"
    def __radd__(self, lhs):
        print "sub_class radd"
sub = sub_class((2,2))
np.float32(42.0) + sub

The problem is line 282 in number.c where a branch uses PyArray_Check() to check the right-hand-side operand m2. If PyArray_Check(m2) returns True, which is the case when m2 is a subclass of ndarray, __array_priority__ is ignored.

Now, this is not a problem when the left-hand-side operand is an ndarray since in CPython a subclass always takes precedence over its superclass. However, a NumPy scalar is not a sub-/super-class of ndarray as the example code above illustrates.

There is a very simple fix: change PyArray_Check(m2) to PyArray_CheckExact(m2). However, this fix has the side-effect that the operation will fail with NotImplementedError if the subclass doesn’t implement the operator, which is a change to the current behavior.
@pv
Copy link
Member
pv commented Jun 2, 2014

Does that change break gh-3502, gh-3503, or gh-3375?

@pv
Copy link
Member
pv commented Jun 2, 2014

__array_priority__ initially existed only for the umath machinery to decide what __array_wrap__ and __array_prepare__ routines it should call, and which array subtype should be returned.

Unfortunately, there is the second problem (as explained in gh-4709) that Python's operator override system has difficulties with the case superclass + two subclasses --- ie. what should the superclass __op__ do to allow sub1 op sub2 and sub2 op sub1 work as expected?

In principle, this problem has nothing to do with __array_priority__ at all --- it's a generic Python programming best-practices question (but one where the best practice seems very unclear). Some of the code paths were later on adapted to use __array_priority__, but as noted in gh-4709 problems still crop up. It seems to me that just the change from Check to CheckExact fails to address some of the issues encountered in gh-4709...

@njsmith
Copy link
Member
njsmith commented Jun 2, 2014

Does numpy_ufunc solve your original problem that prompted the bug
report?
On 2 Jun 2014 11:03, "Mads R. B. Kristensen" notifications@github.com
wrote:

The following code does not output "sub_class radd" as expected since
NumPy ignores array_priority when the right-hand-side operand is a
subclass of ndarray.

import numpy as npclass sub_class(np.ndarray):
array_priority = 1000
def add(self, rhs):
print "sub_class add"
def radd(self, lhs):
print "sub_class radd"sub = sub_class((2,2))np.float32(42.0) + sub

The problem is line 282 in number.c
https://github.com/numpy/numpy/blob/master/numpy/core/src/multiarray/number.c#L282
where a branch uses PyArray_Check() to check the right-hand-side operand
m2. If PyArray_Check(m2) returns True, which is the case when m2 is a
subclass of ndarray, array_priority is ignored.

Now, this is not a problem when the left-hand-side operand is an ndarray
since in CPython a subclass always takes precedence over its superclass.
However, a NumPy scalar is not a sub-/super-class of ndarray as the example
code above illustrates.

There is a very simple fix: change PyArray_Check(m2) to
PyArray_CheckExact(m2). However, this fix has the side-effect that the
operation will fail with NotImplementedError if the subclass doesn’t
implement the operator, which is a change to the current behavior.

—
Reply to this email directly or view it on GitHub
#4766.

@pv
Copy link
Member
pv commented Jun 2, 2014

@njsmith: it will probably solve it, but the execution will end up calling it, due to the aggressive implementation of the base class binop

@madsbk
Copy link
Contributor Author
madsbk commented Jun 3, 2014

@pv: Actually, I believe that changing Check to CheckExact will fix the issue in gh-4709 where overloading __eq__ doesn’t capture np.int64(10) == q when combined with your commit da40eba. However, I do not know if it address the issues with masked arrays and matrices but I would be very surprised if it makes it worse.

Anyhow, using __numpy_ufunc__ could solve my issue if both ndarray and all scalar types call __numpy_ufunc__.
Note that in order for __numpy_ufunc__ to be called by all scalar types, the two _convert_to_ctype() functions in scalarmathmodule.c.src should return -2 (line 559 and 611). The user can trigger this by assigning a __array_priority__ greater than NPY_PRIORITY or by making sure that PyArray_ScalarFromObject() fails. In my opinion, a more elegant solution is simply to use the standard ufunc implementation instead of having a specialized scalar-scalar implementation.

Thanks

@pv pv mentioned this issue Jun 19, 2014
Closed
@DanielSank DanielSank mentioned this issue Jul 30, 2015
Closed
@seberg seberg added 57 - Close? Issues which may be closable unless discussion continued and removed 57 - Close? Issues which may be closable unless discussion continued labels Jan 5, 2019
@madsbk madsbk closed this as completed Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@pv @seberg @njsmith @madsbk
0