-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
ENH: stats.rankdata: add array API standard support #20639
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.
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
Conversation
array-api-compat (and maybe the array API itself) isn't quite ready for this. We'll re-asses after the next array-api-compat release. |
How come this is closed? |
This PR had to add some shims for things which have only recently been added to the standard like In theory we could merge this with the shims, but I suppose it was judged better to prioritise other things from a developer-churn perspective. |
Thanks for the explanation. I see why the PR isn't going to be included (or specifically, isn't efficient to include given the bigger architectural picture). I guess I'm looking for the issue to keep track of the |
If you keep watching this PR, we'll try to make some noise here when work is resumed on |
I think this is possible to revive now that the standard has |
Thanks @mdhaber ! Looking forward to |
I'm guessing it will be ready in 1.17 if all goes well. |
Failures are unrelated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few optional comments but otherwise all good I think
Probably should merge main and run CI as imagine there might be changes needed for #22949 etc
Thanks @j-bowhay. Would you test locally and see if you're getting an error with JAX (when you remove it from the list of incompatible backends)? I haven't seen that one before. |
@j-bowhay Looks like I needed to set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks Matt!
Co-authored-by: Matt Haberland <mhaberla@calpoly.edu>
Thanks @j-bowhay @lucascolley for the reviews! |
I somewhat agree, although it really depends on how widespread use of |
Sure. Just thought I'd note that the failure with identical axes in |
* ENH: stats.rankdata: draft array API support * TST: stats.rankdata: add array API support * TST: stats: fix failing tests * MAINT: stats.rankdata: modernizations * MAINT: stats.rankdata: updates per self-review / CI results * MAINT: stats.rankdata: revisions per review * MAINT: stats.rankdata: run tests with jax_jit=False * Update scipy/stats/_stats_py.py Co-authored-by: Matt Haberland <mhaberla@calpoly.edu> --------- Co-authored-by: Lucas Colley <lucas.colley8@gmail.com>
Reference issue
Towards gh-20544
What does this implement/fix?
Adds array API support to
scipy.stats.rankdata
.Additional information
No need to review this yet. While working on it, I found that it needs several things that aren't covered by
array_api_compat
yet (e.g.repeat
,moveaxis
, NumPyasarray
withcopy=False
), and it would also really benefit from the array API supportingtake_along_axis
(data-apis/array-api#177) anddiff
(data-apis/array-api#791). Also, withoutput
(or preferablyput_along_axis
), this needed a secondargsort
. As-is, this would introduce a substantial performance regression.