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

Add new cloud standard name rule, and update cloud_area_fraction #37

Merged
merged 7 commits into from
Jun 26, 2023

Conversation

nusbaume
Copy link
Collaborator

This pull request adds a new rule specifying that unless stated otherwise, the word cloud means all cloud phases and types.

Also, it was brought up by @grantfirl That the standard name cloud_area_fraction as used in the UFS doesn't include convective clouds, so the standard name was updated, along with standard names that appeared directly related, to specify that convective clouds are not included.

Finally, I am not strongly tied to the specific wording of either the new rule or the updated standard names, and so am happy to change them to whatever people prefer.

StandardNamesRules.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@grantfirl grantfirl left a comment

Choose a reason for hiding this comment

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

I'd prefer non_convective be changed to either nonconvective or non-convective. I've seen it hyphenated or combined, but never separated with a space, which is what the underscore is representing. Also, I think that we're probably going to need a more general variable at some point that includes all clouds (including convective).

@nusbaume
Copy link
Collaborator Author

@grantfirl That's a good point! I've gone ahead and replaced non convective with non-convective. Also I agree that having an "all cloud" variable standard name will be required, but I figured that would be added as soon as someone explicitly needed it. However if you would like me to go ahead and add one now then just let me know.

@nusbaume nusbaume requested a review from grantfirl June 13, 2023 17:14
@@ -354,7 +354,9 @@
<standard_name name="mass_content_of_cloud_liquid_water_in_atmosphere_layer">
<type kind="kind_phys" units="kg m-2">real</type>
</standard_name>
<standard_name name="cloud_area_fraction_in_atmosphere_layer">
<standard_name
name="non-convective_cloud_area_fraction_in_atmosphere_layer"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This violates the Standard Name Rules.
Rule 2 states that standard names must follow the CF metadata rules for constructing standard names. This document states:
"Standard names consist of lower-letters, digits and underscores, and begin with a letter. Upper case is not used."
Please remove dashes in standard names.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gold2718 Good catch! I've gone ahead and removed the dash from the standard names.

Also, given that I am probably not going to be the only one who violates rule 2, I wonder if it would be worth adding a Github Action that checks that all of a PR's standard names are compliant? I'll go ahead and make a new issue so folks can discuss it there.

@nusbaume nusbaume requested a review from gold2718 June 13, 2023 17:56
Copy link
Collaborator

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

Looks okay to me now.

@nusbaume nusbaume merged commit d7d77b0 into ESCOMP:main Jun 26, 2023
@nusbaume nusbaume deleted the cloud_rules branch November 9, 2023 17: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.

4 participants