-
Notifications
You must be signed in to change notification settings - Fork 19
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
Data.mask
: migrate to Dask
#301
Data.mask
: migrate to Dask
#301
Conversation
Hi Sadie, I'm afraid that I favour the existing behaviour, which I also see as being consistent with numpy. For a numpy array The following also works: >>> import cf
>>> d = cf.example_field(0).data
>>> a = d.mask.array
>>> (a == d.mask).all()
True i.e. we don't need the |
Hi @davidhassell, fair enough, thanks for explaining the context - I think in hindsight I was getting confused over the nature of |
422aa6e
to
e3c926c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Sadie, thanks - I've suggested a different tack on the main method, we can discuss this afternoon, perhaps.
Co-authored-by: David Hassell <[email protected]>
Note: did not combine with 'test_Data_apply_masking' at this stage as that isn't passing yet; separate test makes it easier to test the migrated method.
All feedback has now been addressed. Triggering the CI jobs via open-close for a final sanity check... |
All good: local test passes, CI jobs pass, & all feedback has been addressed, including adding a new unit test. Clearance to merge assuming similar conditions from David given externally prior to his taking leave, so merging now. |
Convert the
mask
managed attribute towards #182.