-
Notifications
You must be signed in to change notification settings - Fork 17
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 CCPP metadata file standard name checking script #47
Conversation
…ing works with the new repo structure.
@cacraigucar I took you off the reviewer list because it's basically all python code, but if you would still like to review this PR then just let me know. Thanks! |
Apologies for the sudden flurry of commits, but of course I found a bug as soon as I opened the PR. It should be resolved now, and so this PR is ready to review whenever. |
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.
Looks good, just some optional comments. Funnily enough, I randomly chose a CCPP physics meta file to test this new script (ufs-weather-model/FV3/ccpp/physics/physics/rrtmgp_sw_main.meta
), and it has a lot of non-standard names. I was actually looking for a case that had all valid standard names for comparison and couldn't find one 😬
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.
@nusbaume Thanks for creating this script. Looks good to me.
As for using this for testing, I think it would be deployed outside of this repository? For example, in the ccpp-physics repositories and host-model repositories there could be CI tests to run this script and compare their metadata to the standard_name repository?
Just FYI that I added a timestamp to the printed output as requested by @cacraigucar and @peverwhee. |
@dustinswales Apologies for missing your post! Yes, I was assuming this would be a tool one could apply to CCPP metadata files in external repos that contain such files (e.g. I certainly think that long-term it would be good to implement something like this script in a CI test to ensure that all standard names listed in a new metadata file are in the dictionary. However, I haven't really pushed for it at this stage simply because I believe a lot of physics schemes don't have fully dictionary-compliant standard names, and that it will probably take some work before a critical mass of them do. |
@nusbaume No worries (I forgot that I made a comment...) |
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.
One comment that you can take or leave.
Thanks Jesse!
This Pull Request adds a new
meta_stdname_check.py
script that takes a path either to a metadata file or a directory and attempts to parse all of the found metadata files to see if there are any standard names that do not exist in the provided standard names dictionary, and then attempts to print the list to stdout in a hopefully easy-to-read way.Along with this, I moved all of the other python files into a new
tools
andlib
directory, to try and reduce the flatness of the repo. This also required me to modify some of these scripts, as well as the Github Action workflow file, to make sure everything still works the way it is expected to.Finally, I should note that I manually tested all of the scripts, but haven't implemented any sort of formal testing of the scripts themselves. If this is desired then please let me know.
Fixes #41