Skip to content

Aggregation of fields referencing external variables#172

Merged
sadielbartholomew merged 8 commits intoNCAS-CMS:masterfrom
sadielbartholomew:external-cell-measures
Dec 17, 2020
Merged

Aggregation of fields referencing external variables#172
sadielbartholomew merged 8 commits intoNCAS-CMS:masterfrom
sadielbartholomew:external-cell-measures

Conversation

@sadielbartholomew
Copy link
Copy Markdown
Member

@sadielbartholomew sadielbartholomew commented Dec 17, 2020

Implement agreed behaviour for the aggregation of field constructs having at least one reference to an external netCDF variable to define cell measure construct(s) (the only possibility allowed by the current status of the CF Conventions), namely to aggregate such fields otherwise aggregatable but to delete the external cell measure on the aggregated output.

This is sensible behaviour that will be sufficient for the next release, 3.8.0, but we decided it would be good to review the meaning of external cell measures towards a consistent and well-defined treatment in future (see Issue to be raised and linked to this PR).

Note this closes #150 (see in particular the comment here) since it enables the fields there to be aggregated to one.

@sadielbartholomew sadielbartholomew self-assigned this Dec 17, 2020
@sadielbartholomew sadielbartholomew changed the title Sensible aggregation of fields referencing external variables Aggregation of fields referencing external variables Dec 17, 2020
Copy link
Copy Markdown
Collaborator

@davidhassell davidhassell left a comment

Choose a reason for hiding this comment

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

This looks very good to me. We should hold off merging just to conclude the discussion on https://github.com/NCAS-CMS/cf-python/pull/172/files#r544996379.

@sadielbartholomew
Copy link
Copy Markdown
Member Author

sadielbartholomew commented Dec 17, 2020

Hi @davidhassell, I have just finalised the PR to take the revised approach as you suggested in the comment above. A test was amended slightly to account for the new behaviour (the field split into three parts with one having had the external measure deleted will aggregate to a single field rather than two that would in turn aggregate to one if run through cf.aggregate again, as desired.) So if you are happy this is ready to merge.

Sorry for the delay, since the Telco I have been working on the append mode PR (locally) in parallel...

@sadielbartholomew
Copy link
Copy Markdown
Member Author

(Since my comment above I saw I had one changelog conflict to resolve, so have just done that.)

@davidhassell
Copy link
Copy Markdown
Collaborator

Looks good, thanks - merge away!

@sadielbartholomew sadielbartholomew merged commit 2cdce3d into NCAS-CMS:master Dec 17, 2020
@sadielbartholomew sadielbartholomew deleted the external-cell-measures branch December 17, 2020 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

How does cf.aggregate work?

2 participants