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

Public testing framework for duck array integration #6894

Open
3 of 19 tasks
TomNicholas opened this issue Aug 8, 2022 · 8 comments
Open
3 of 19 tasks

Public testing framework for duck array integration #6894

TomNicholas opened this issue Aug 8, 2022 · 8 comments
Labels
array API standard Support for the Python array API standard enhancement topic-arrays related to flexible array support topic-testing

Comments

@TomNicholas
Copy link
Contributor

TomNicholas commented Aug 8, 2022

What is your issue?

In #4972 @keewis started writing a public framework for testing the integration of any duck array class in xarray, inspired by the testing framework pandas has for ExtensionArrays. This is a meta-issue for what our version of that framework for wrapping numpy-like duck arrays should look like.

(Feel free to edit / add to this)

What behaviour should we test?

We have a lot of xarray methods to test with any type of duck array. Each of these bullets should correspond to one or more testing base classes which the duck array library author would inherit from. In rough order of increasing complexity:

We don't need to test that the array class obeys everything else in the Array API Standard. (For instance .device is probably never going to be used by xarray directly.) We instead assume that if the array class doesn't implement something in the API standard but all the generated tests pass, then all is well.

How extensible does our testing framework need to be?

To be able to test any type of wrapped array our testing framework needs to itself be quite flexible.

  • User-defined checking - For some arrays np.testing.assert_equal is not enough to guarantee correctness, so the user creating tests needs to specify additional checks. Automatic duck array testing - reductions #4972 shows how to do this for checking the units of resulting pint arrays.
  • User-created data? - Some array libraries might need to test array data that is invalid for numpy arrays. I'm thinking specifically of testing wrapping ragged arrays. Awkward array backend? #4285
  • Parallel computing frameworks? - Related to the last point is chunked arrays. Here the strategy requires an extra chunks argument when the array is created, and any results need to first call .compute(). Testing parallel-executed arrays might also require pretty complicated SetUps and TearDowns in fixtures too. (see also Alternative parallel execution frameworks in xarray #6807)

What documentation / examples do we need?

All of this content should really go on a dedicated page in the docs, perhaps grouped alongside other ways of extending xarray.

  • Motivation
  • What subset of the Array API standard we expect duck array classes to define (could point to a typing protocol?)
  • Explanation that the array type needs to return the same type for any numpy-like function which xarray might call upon that type (i.e. the set of duckarray instances is closed under numpy operations)
  • Explanation of the different base classes
  • Simple demo of testing a toy numpy-like array class
  • Point to code testing more advanced examples we actually use (e.g. sparse, pint)
  • Which advanced behaviours are optional (e.g. Constructors and Properties have to work, but Groupby is optional)

Where should duck array compatibility testing eventually live?

Right now the tests for sparse & pint are going into the xarray repo, but presumably we don't want tests for every duck array type living in this repository. I suggest that we want to work towards eventually having no array library-specific tests in this repository at all. (Except numpy I guess.) Thanks @crusaderky for the original suggestion.

Instead all tests involving pint could live in pint-xarray, all involving sparse could live in the sparse repository (or a new sparse-xarray repo), etc. etc. We would set those test jobs to re-run when xarray is released, and then xref any issues revealed here if needs be.

We should probably also move some of our existing tests #7023 (review)

@keewis
Copy link
Collaborator

keewis commented Aug 9, 2022

with the implementation in #4972 you should already be able to specify a hypothesis strategy to create e.g. a random awkward array. Same with dask or other parallel computing frameworks: if you can construct a hypothesis strategy for them the testing framework should be able to use that. check_reduce (or maybe it should be just check?) should allow customizing the comparison (or actually, that's the entire test code at the moment), so putting compute (or todense / get) calls should be easy.

For setup and teardown I think we could use pytest fixtures (and apply them automatically to each function). However, maybe we should just not use parametrize but instead define separate functions for each reduce operation? Then it would be possible to override that manually. As far as I remember I chose not to do that because tests that only delegate to super().test_function() just are not great design – if we can think of a way to do that while avoiding those kinds of test redefinitions I'd be happy with that (and then we could get rid of the apply_marks function which is a ugly hack of pytest internals).

I agree that moving the array library tests to dedicated repositories makes a lot sense (for example, the pint tests use old versions of the conversion functions from pint-xarray), but note that at the moment the tests for pint seem to increase the total test coverage of xarray a bit. I guess that just means we'd have to improve the rest of the testsuite?

@TomNicholas
Copy link
Contributor Author

you should already be able to specify a hypothesis strategy to create e.g. a random awkward array

Sounds good!

or maybe it should be just check?

Yes just check probably.

However, maybe we should just not use parametrize but instead define separate functions for each reduce operation?

But then the user writing the test code would have to write one of their own tests per xarray method wouldn't they? I think we should avoid putting that much work on them if we can. I think your current approach seems fine so far...

the pint tests use old versions of the conversion functions from pint-xarray

That's basically technical debt, so we should make a point to remove them from xarray eventually.

the tests for pint seem to increase the total test coverage of xarray #5692 (comment). I guess that just means we'd have to improve the rest of the testsuite?

So long as @benbovy (or someone) writes new tests to cover the bugs that were revealed then this is fine.

@Illviljan
Copy link
Contributor

Typing duck array is also a little challenging I find, we pretty much only do Any at the moment. I found some nice references and discussions that might be interesting for this:
https://github.com/pmeier/array-protocol
data-apis/array-api#229

@TomNicholas
Copy link
Contributor Author

Typing duck array is also a little challenging I find

Thanks @Illviljan - I was literally just thinking about that here.

@TomNicholas
Copy link
Contributor Author

Another thing that might be useful is the hypothesis strategies in the test suite for the array API consortium standard (cc @keewis).

@keewis
Copy link
Collaborator

keewis commented Aug 16, 2022

there's also the experimental array api strategies built into hypothesis

@jhamman
Copy link
Member

jhamman commented Sep 22, 2022

@asmeurer recently pointed me to https://data-apis.org/array-api-tests/. Would that be useful here?

@TomNicholas
Copy link
Contributor Author

Looks like these

https://data-apis.org/array-api-tests/.

use these

experimental array api strategies

Would that be useful here?

I think they are complementary. In theory if xarray supports the array API standard and a library passes all the data array API tests, then it should also pass all of xarray's tests (rendering the latter uneccessary). But in practice I think the tests here would still be useful, if only for the possible case of libraries that don't fully meet the API standard yet would still work fine in xarray.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
array API standard Support for the Python array API standard enhancement topic-arrays related to flexible array support topic-testing
Projects
None yet
Development

No branches or pull requests

4 participants