Skip to content
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

[DO NOT MERGE] Aggregation variables first draft #9

Closed
wants to merge 100 commits into from

Conversation

davidhassell
Copy link
Owner

See issue #XXX for discussion of these changes.

Release checklist

  • Authors updated in cf-conventions.adoc?
  • Next version in cf-conventions.adoc up to date? Versioning inspired by SemVer.
  • history.adoc up to date?
  • Conformance document up-to-date?

For maintainers

After the merge remember to delete the source branch.
Tags are set at the conclusion of the annual meeting; until then master always is a draft for the next version.

castelao and others added 28 commits February 2, 2024 09:59
I used the authors list from version v1.10. I don't know affiliations or
ORCIDs, but it's better anyways each one to add yours.
As discussed in the meeting today.
My branch was behind with the official main creating some confusion with
@larsbarring's PR. I rebased my branch, and I'm adding here his changes.
Co-authored-by: Sadie L. Bartholomew <[email protected]>
Copied from cf-conventions website.
Copied from CF-Conventions website.
@ethanrd and @JonathanGregory , I just noticed that @kevin-obrien is missing on
the contributors' list
(https://github.com/cf-convention/cf-convention.github.io/blob/main/conventions_contributors.md)

@kevin-obrien, could you double check your affiliation and orcid here,
please?
It looks like I was using the wrong label according to:
https://spdx.org/licenses/CC0-1.0.html
Co-authored-by: Ethan Davis <[email protected]>
@taylor13 , could you verify if it, please?
@japamment, could you double check it, please?
Co-authored-by: Martin Raspaud <[email protected]>
@davidhassell davidhassell changed the title Aggregation variables first draft [DO NOT MERGE] Aggregation variables first draft Apr 18, 2024
Copy link

@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 am very happy with this. Thanks for addressing my previous feedback. Ive raised some final minor comments but nothing substantial save for perhaps the need to clarify whether packing is forbidden or just advised against (see in-line comment). Please go ahead and raise formally to the CF Conventions when you've considered those.

appa.adoc Outdated Show resolved Hide resolved
history.adoc Outdated Show resolved Hide resolved
ch02.adoc Outdated Show resolved Hide resolved
ch02.adoc Outdated Show resolved Hide resolved
default-theme-CF-version.yml Outdated Show resolved Hide resolved
conformance.adoc Outdated Show resolved Hide resolved
appl.adoc Outdated Show resolved Hide resolved
davidhassell and others added 22 commits May 14, 2024 10:09
Co-authored-by: Sadie L. Bartholomew <[email protected]>
Co-authored-by: Sadie L. Bartholomew <[email protected]>
Co-authored-by: Sadie L. Bartholomew <[email protected]>
Co-authored-by: Sadie L. Bartholomew <[email protected]>
Co-authored-by: Sadie L. Bartholomew <[email protected]>
Co-authored-by: Sadie L. Bartholomew <[email protected]>
Initializing a descriptor for Zenodo's DOI
clarify the recommendation to use the convention of 4.3.3 for parametric vertical coordinates
…ension, attribute and group names is a recommendation, not a requirement
Clarify that the character set given in section 2.3 for variable, dimension, attribute and group names is a recommendation, not a requirement
Fix typo 'principal' -> 'principle' in ch02
@dwest77a
Copy link

Comments on wording/syntax:

  • Section 2.8 paragraph 3: Suggest changing to An aggregation variable must **present as** a scalar **within the aggregation file**
  • Section 2.8 paragraph 5 point 1: Suggest changing to The dimension of a coordinate variable of an aggregation data variable must be **included as** one of the aggregated dimensions of the aggregation variable.

@davidhassell
Copy link
Owner Author

Thanks, @dwest77a, I'll take the second one :) 5efc8f9

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.

10 participants