Automatic duck array testing - reductions #4972
Conversation
|
Q: Shouldn't the base classes live in Then the |
| def check_reduce(self, obj, op, *args, **kwargs): | ||
| actual = getattr(obj, op)(*args, **kwargs) | ||
|
|
||
| data = np.asarray(obj.data) |
There was a problem hiding this comment.
Could we rename instances of arrays to be called arr instead of data? It took me a pretty long time to realise that this had nothing to do with hypothesis' strategies.data.
There was a problem hiding this comment.
sure, although it would probably be better to call it numpy_arr or move it into the .copy call
Another Q on a similar note: Are we planning to eventually publicly expose the (awesome btw) strategies that you've built here @keewis ? They could be very useful for testing other parts of xarray. We could also make this PR much more incremental by splitting it into 2, or even 3 separate PRs:
The advantage of that would be that (1) & (2) can move forwards without requiring all the tests in (3) to pass. |
There was a problem hiding this comment.
yes! This PR is just a prototype of how the testing framework could look like, I didn't really think about the structure yet. I also agree that both the strategies as well as the base classes should eventually live in xarray.testing, but I'd probably keep them in xarray.tests for now (just so we can experiment a bit more).
Note, however, that it's probably best not to separate 2 and 3 because there might be issues with the base class design we don't notice without trying to actually use them.
| def check_reduce(self, obj, op, *args, **kwargs): | ||
| actual = getattr(obj, op)(*args, **kwargs) | ||
|
|
||
| data = np.asarray(obj.data) |
There was a problem hiding this comment.
sure, although it would probably be better to call it numpy_arr or move it into the .copy call
So I may actually have overexcitedly already jumped the gun and made a PR to move strategies to
Good point. |
TomNicholas
added
topic-testing
topic-arrays
I actually think it should be the other way around: if we can get the strategies from #6908 to shrink well, we might be able to fix the occasional test timeouts here (which should be one of the final issues we have before we can merge this). |
I think they already do shrink well. Each of them has corresponding unit tests, and none of those tests fail due to hypothesis timeouts. |
This is the first of a series of PRs to add a framework to make testing the integration of duck arrays as simple as possible. It uses
hypothesisfor increased coverage and maintainability.pre-commit run --all-fileswhats-new.rstapi.rst