Skip to content

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'

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