API: Introduce correction argument for np.var and np.std [Array API]
#25169
Conversation
7b6e406 to
a4aa82f
Compare
a4aa82f to
d776e42
Compare
d776e42 to
2fb6d10
Compare
|
One of the jobs failed with indicating a possible aarch64 mac heisenbug. Commenting here for reference in case it happens again. Restarted both failing CI jobs which are unrelated I think. |
There was a problem hiding this comment.
This looks good to me, although this was one of the array API changes that was flagged at the last triage meeting as one that might be controversial because people are not a fan of the name correction.
I personally think it's fine to have duplicate names for better array API compatibility but I'm holding off on merging this until we can have a deeper community discussion around the coming NEP for this.
correction argument for np.var and np.stdcorrection argument for np.var and np.std [Array API]
There was a problem hiding this comment.
I cannot say I like the name correction much, but given its existence, this s
10000
eems the right way forward. Some comments in-line.
numpy/_core/fromnumeric.py
Outdated
| @array_function_dispatch(_std_dispatcher) | ||
| def std(a, axis=None, dtype=None, out=None, ddof=0, keepdims=np._NoValue, *, | ||
| where=np._NoValue, mean=np._NoValue): | ||
| where=np._NoValue, mean=np._NoValue, correction=0): |
There was a problem hiding this comment.
Should we make the default correction=_np.NoValue so that we can truly recognize if some value is put in? Could even consider doing the same for ddof, but that would be an API change for people whose classes have a .std() method, so probably best not.
There was a problem hiding this comment.
I like this idea - done! Then std(x, ddof=1, correction=0) can also be recognized as incorrect set of parameters.
(updated only correction parameters)
|
|
||
| if type(a) is not mu.ndarray: | ||
| try: | ||
| std = a.std |
There was a problem hiding this comment.
Good reminder: any class that defines .std() gets these arguments passed on. So, there is a choice to be made here: do we pass on correction (and not ddof) if it is passed in, or do we, as implemented now, just set ddof=correction. I think the choice here is fine, but raising the possible issue just in case (I have had to implement .std() in some astropy classes, but the choice here would not prevent me from adding correction; I'd still need to support ddof anyway...).
There was a problem hiding this comment.
Thank you for pointing it out. I also think that the current setup is fine: Array API defines std and var only for the main namespace. There's no std and var for the Array Object, so there's no need to introduce correction in numpy.ndarray class.
There was a problem hiding this comment.
Marten's point is a little bit different though, this path only happens if the array-like is explicitly not an ndarray. That said, he's also right that any array-like implmenting a std method would need to deal with ddof for backward compatibility, so this code is fine.
If, in the future, we ever decide to deprecate ddof in favor of correction, this issue becomes a bit more acute, but we could always do hacks like first pass correction in a try/except block, see if there's an error, and if so try again with ddof.
correction argument for np.var and np.std [Array API]correction argument for np.var and np.std [Array API]
2fb6d10 to
4540923
Compare
|
Alright, I think this one is ready to go in since there haven't been any followups since the last updates. Thanks @mtsokol! |
This PR contains next Array API compatibility change.
It adds
correctionargument tonp.varandnp.std(and alsonp.nanvarandnp.nanstd), which is an Array API compatible name forddof.