Skip to content
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

ENH: add testing support utilities #17

Open
ev-br opened this issue Nov 7, 2024 · 7 comments
Open

ENH: add testing support utilities #17

ev-br opened this issue Nov 7, 2024 · 7 comments
Labels
enhancement New feature or request new function

Comments

@ev-br
Copy link
Contributor

ev-br commented Nov 7, 2024

SciPy has two sets of testing support utilities:

@lucascolley
Copy link
Member

These utilities are specific to the libraries we test with in SciPy, so I don't think they would fit into the current scope of this library.

I think it may be possible to generalise {skip_xfail}_xp_backends over devices and skip based on https://data-apis.org/array-api/draft/API_specification/generated/array_api.info.capabilities.html rather than individual backends. But we would probably still want something separate in SciPy which is able to deal with specific implementations.

Since there are no xp.testing primitives in the standard, I don't see how we would add xp_assert_{equal,close} in an agnostic way.

I agree that these utilities should be pulled out of SciPy into some package which other consuming libraries can re-use. But I think that it would be useful to keep them separate from array-api-extra to avoid confusion, by keeping this library as purely building on top of the standard.

@asmeurer
Copy link
Member

asmeurer commented Nov 7, 2024

I discussed this with Evgeni a few weeks ago, and based on his description of the SciPy tooling, I suggested that these utilities be split out into a separate package that is a pytest extension (e.g., called something like pytest-arrays or pytest-array-api).

@lucascolley lucascolley added out-of-scope? enhancement New feature or request labels Nov 7, 2024
@ev-br
Copy link
Contributor Author

ev-br commented Nov 18, 2024

To gauge interest of other downstream adopters: @betatim would you consider using scipy's @skip_xp_backends machinery and/or backend-agnostic assertions, xp_assert_{close,equal} in your scikit-learn work if they were in a separate package, as suggested above?
(We can take this discussion elsewhere if that'd be more convenient)

@betatim
Copy link
Member

betatim commented Nov 18, 2024

A bit tricky to judge in the abstract.

Right now we convert all outputs back to Numpy arrays and then use the "standard" all close, equal, etc tooling. So in principle having a xp_assert_close could be nice, but in practice we have already solved this problem. And the current implementation isn't causing us any trouble - don't change what ain't broken?

For skipping particular namespaces: we use a decorator to generate combinations of xp, dtype, device and for now all tests run on all combinations (I think). So for now maybe there is no need for skip_xp_backends for scikit-learn?

@crusaderky
Copy link
Contributor

#88 adds xp_assert_equal and xp_assert_close to array-api-extra, for private use only, at least for now.

The xpx version misses a bunch of bells and whistles that the scipy version has:

  • no default_ctx context
  • no ability to cherry-pick disabling shape, dtype, and/or namespace validation
  • no special casing for 0d

I was keen to avoid scope creep in #88, so I stuck to the mvp for array-api-extra itself.
Feel free to expand them to be able to replace those in scipy or move the whole thing to a separate package (which would become a test dependency of xpx, I guess).

@lucascolley
Copy link
Member

also, since I commented above, the scope of xpx has expanded, so these are probably no longer out of scope.

@ev-br
Copy link
Contributor Author

ev-br commented Jan 14, 2025

If anything, ISTM it'd be best to move xp_assert functions wholesale, as they are. 0d versions can either move or stay, of course. Not having two nearly but not quite identical functions in two packages is best avoided though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request new function
Projects
None yet
Development

No branches or pull requests

5 participants