Optional tolerance feature to isin per issue #5587 #5862
Conversation
|
Hello @shazelquist! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-10-23 00:24:37 UTC |
xarray/tests/test_duck_array_ops.py
Outdated
|
|
||
| @pytest.mark.parametrize("shape", [(200, 1), (10, 10, 2), (4, 50)]) | ||
| @pytest.mark.parametrize("tolerance", [1e-2, 1e-4, 1e-6]) | ||
| def test_isin_tolerance(shape, tolerance): |
There was a problem hiding this comment.
Can we add a test with dask arrays as well?
There was a problem hiding this comment.
Sure! Does the implementation in commit b79620a cover dask arrays?
There was a problem hiding this comment.
You have to change the expected value as well. Because if I input a dask array I would expect to get a dask array out as well from a duckarray operation.
There's also a assert_duckarray_equal that you maybe can use instead of numpys version:
Line 215 in b79620a
I'm probably missing something but it is kind of surprising it hasn't already been imported in this test module.
There was a problem hiding this comment.
Oh yeah, that makes sense. Thanks for the clarification!
shazelquist
force-pushed
the
tolerance_for_isin
branch
from
ebe3e32 to
b2e32aa
Compare
da.isin()? #5587pre-commit run --all-fileswhats-new.rstNew functions/methods are listed indoesn't appear to be necessaryapi.rstThis implementation is similar to suggested vectorized solution 1, but acts more according to the issue specifications.
I'm pretty this pull fails checks 4 and 5, but they may not be relevant to expanding functions from the
apply_ufuncdispatcher?I will write an update if this is the case or any other changes are required.