-
Notifications
You must be signed in to change notification settings - Fork 20
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.__init__
#262
dask: Data.__init__
#262
Conversation
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.
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.
# TODODASK: When PR #257 (dask: Data.__getitem__, | ||
# Data.__setitem__) is merged, add in a | ||
# "reset_mask_hardness=False". |
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.
#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.)
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. |
No description provided.