Skip to content

Conversation

@davidhassell
Copy link
Collaborator

No description provided.

@davidhassell davidhassell added the dask Relating to the use of Dask label Sep 30, 2021
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.

Perfect, thanks. I'll address the post-#257 TODO now.

As a side note, the full test suite is running on Actions when it shouldn't be; only the six 'Test cf.Data during the replacement of LAMA with Dask' jobs should be running (as they have for other PRs). I'll look into why that is. For this PR, we can ignore the failures as they are of course expected until daskification is much more mature. The six test jobs of relevance pass on the CI here, and test_Data passes for me locally, so this is all good to merge.

Comment on lines +479 to +481
# TODODASK: When PR #257 (dask: Data.__getitem__,
# Data.__setitem__) is merged, add in a
# "reset_mask_hardness=False".
Copy link
Member

Choose a reason for hiding this comment

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

#257 is now in, and so I can get this merged now to clear the reviewing backlog I will address this TODO and add in reset_mask_hardness=False myself as a follow-up commit. (I hope and assume that's OK.)

@sadielbartholomew
Copy link
Member

There was a single trivial conflict to resolve, so I've went ahead and done it (see latest commit) so I can merge this now.

@sadielbartholomew sadielbartholomew merged commit fc11e97 into NCAS-CMS:lama-to-dask Oct 6, 2021
sadielbartholomew added a commit that referenced this pull request Oct 7, 2021
@davidhassell davidhassell deleted the dask-init branch November 15, 2022 09:11
@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