Enable clip broadcasting, with apply_ufunc #5184
Conversation
|
The tests are in a reasonable state — not perfect — but maybe good enough to proceed. Are there any cases that are missing currently? |
| @@ -1665,6 +1665,11 @@ def fillna(self, value): | |||
| def where(self, cond, other=dtypes.NA): | |||
| return ops.where_method(self, cond, other) | |||
|
|
|||
| def clip(self, min=None, max=None): | |||
There was a problem hiding this comment.
I wonder whether there should be a class that DataArray & Variable should inherit from (but not AbstractArray) where they share methods.
I don't think there's enough overlap to justify it at the moment, but worth considering. That may mean we could unify the tests, which are currently highly duplicated for clip for example.
Co-authored-by: Deepak Cherian <[email protected]>
|
Hello @max-sixty! 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-04-19 18:17:58 UTC |
They're there! The magic of fixtures is Does that make sense, assuming I'm not missing anything? Too magical? |
pre-commit run --all-fileswhats-new.rstI think this is the right approach — we may need to confirm the appropriate args to
apply_ufunc— probably we want the same as.where?Tests are OK but a bit lacking atm.