LAMA to Dask: misc. fixes for a passing `test_aggregate` by sadielbartholomew · Pull Request #572 · NCAS-CMS/cf-python
Towards #182, confirms that cf.aggregate works under the new "Dask way" with no modifications, with test_basic_aggregate failing instead due to specific issues elsewhere, namely:
- an issue with data comparison by
Data.equals; an issue with construct comparison leading to field copies not being recognised as equal.
(Note to tidy up previous investigation/debugging branches, I created a fresh branch and re-applied commits to break down the fixes to specific parts.)
@davidhassell please either hold on the reviewing here until tomorrow morning, or you can partially review under the following context:
- as-is on this branch,
test_aggregatepasses fully, but notably I've commented out a set of related test assertions, which rely on a fix in cfdm for which I am preparing a PR but it is not quite ready so I will put up tomorrow morning; - once the corresponding cfdm PR is in, I will self-review that everything's working together and then open this for review fully.
If you like you could review the commits here standalone noting the above, with the main one is the initial tweak to fix an aspect of Data.equals, 02133f7.
Thanks for all of your invaluable guidance here @davidhassell. (I manually added you as a co-author on the new fix approach commit, since you essentially wrote that code!)
This is good to review now, I think. FYI, the linting CI job is failing due to steup issues relating to GitLab (I see these on the command line too, so think it will resolve itself in time and will ignore it for now). So we can ignore that.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. All makes sense following our conversations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters