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

dask: Data.mask_fpe #380

Merged
merged 7 commits into from
Apr 21, 2022
Merged

Conversation

davidhassell
Copy link
Collaborator

No description provided.

@davidhassell
Copy link
Collaborator Author

@sadielbartholomew - just a note that this PR may affect the daskification of _binary_arithmetic.

Copy link
Member

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall:

I have raised a few minor comments but otherwise this is ready to merge, with +1 for a clear and detailed deprecation message.

Notes:

flake8 uncovers that that there is still a reference to _mask_fpe, but as you have implied it is in _binary_operation which I am covering (PR to go up today or tomorrow) so not touching it here to avoid the merge conflict is wise. I've already removed the reference to it in my branch for the PR.

There is one other undefined name though, maybe we can deal with it here whilst we are here? It's in get_filenames which is marked as migrated in the big table:

flake8...................................................................Failed
- hook id: flake8
- exit code: 1

cf/data/data.py:4181:20: F821 undefined name '_mask_fpe'
cf/data/data.py:9406:30: F821 undefined name 'FileArray'

cf/data/mixin/deprecations.py Outdated Show resolved Hide resolved
cf/data/mixin/deprecations.py Outdated Show resolved Hide resolved
davidhassell and others added 3 commits April 20, 2022 14:58
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
@davidhassell
Copy link
Collaborator Author

Thanks, Sadie. The FileArray is annoying, and wholly my fault, but it has now been resolved in other PRs. (Also, I've spotted some holes in get_filenames which need to be revisited ...)

@davidhassell davidhassell merged commit 2ad48d1 into NCAS-CMS:lama-to-dask Apr 21, 2022
@davidhassell davidhassell deleted the dask-mask-fpe branch November 15, 2022 09:21
@davidhassell davidhassell added this to the 3.14.0 milestone Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dask Relating to the use of Dask
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants