23.07.2014, 20:37, Julian Taylor kirjoitti: [clip: __numpy_ufunc__]
So its been a week and we got a few answers and new issues. To summarize: - to my knowledge no progress was made on the issues - scipy already has a released version using the current implementation - no very loud objections to delaying the feature to 1.10 - I am still unfamiliar with the problematics of subclassing, but don't want to release something new which has unsolved issues.
That scipy already uses it in a released version (0.14) is very problematic. Can maybe someone give some insight if the potential changes to resolve the remaining issues would break scipy?
If so we have following choices:
- declare what we have as final and close the remaining issues as 'won't fix'. Any changes would have to have a new name __numpy_ufunc2__ or a somehow versioned the interface - delay the introduction, potentially breaking scipy 0.14 when numpy 1.10 is released.
I would like to get the next (and last) numpy 1.9 beta out soon, so I would propose to make a decision until this Saturday the 26.02.2014 however misinformed it may be.
It seems fairly unlikely to me that the `__numpy_ufunc__` interface itself requires any changes. I believe the definition of the interface is quite safe to consider as fixed --- it is a fairly straighforward hook for Numpy ufuncs. (There are also no essential changes in it since last year.) For the binary operator overriding, Scipy sets the constraint that ndarray * spmatrix MUST call spmatrix.__rmul__ even if spmatrix.__numpy_ufunc__ is defined. spmatrixes are not ndarray subclasses, and various subclassing problems do not enter here. Note that this binop discussion is somewhat separate from the __numpy_ufunc__ interface itself. The only information available about it at the binop stage is `hasattr(other, '__numpy_ufunc__')`. *** Regarding the blockers: (1) https://github.com/numpy/numpy/issues/4753 This is a bug in the argument normalization --- output arguments are not checked for the presence of "__numpy_ufunc__" if they are passed as keyword arguments (as a positional argument it works). It's a bug in the implementation, but I don't think it is really a blocker. Scipy sparse matrices will in practice seldom be used as output args for ufuncs. *** (2) https://github.com/numpy/numpy/pull/4815 The is open question concerns semantics of `__numpy_ufunc__` versus Python operator overrides. When should ndarray.__mul__(other) return NotImplemented? Scipy sparse matrices are not subclasses of ndarray, so the code in question in Numpy gets to run only for ndarray * spmatrix This provides a constraint to what solution we can choose in Numpy to deal with the issue: ndarray.__mul__(spmatrix) MUST continue to return NotImplemented This is the current behavior, and cannot be changed: it is not possible to defer this to __numpy_ufunc__(ufunc=np.multiply), because sparse matrices define `*` as the matrix multiply, and not the elementwise multiply. (This settles one line of discussion in the issues --- ndarray should defer.) How Numpy currently determines whether to return NotImplemented in this case or to call np.multiply(self, other) is by comparing `__array_priority__` attributes of `self` and `other`. Scipy sparse matrices define an `__array_priority__` larger than ndarrays, which then makes a NotImplemented be returned. The idea in the __numpy_ufunc__ NEP was to replace this with `hasattr(other, '__numpy_ufunc__') and hasattr(other, '__rmul__')`. However, when both self and other are ndarray subclasses in a certain configuration, both end up returning NotImplemented, and Python raises TypeError. The `__array_priority__` mechanism is also broken in some of the subclassing cases: https://github.com/numpy/numpy/issues/4766 As far as I see, the backward compatibility requirement from Scipy only rules out the option that ndarray.__mul__(other) should unconditionally call `np.add(self, other)`. We have some freedom how to solve the binop vs. subclass issues. It's possible to e.g. retain the __array_priority__ stuff as a backward compatibility measure as we do currently. -- Pauli Virtanen