-
Notifications
You must be signed in to change notification settings - Fork 36
Update CMIP7 unit tests #884
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
Update CMIP7 unit tests #884
Conversation
…the CV without needing to be listed in required_global_attributes
|
Thanks for the great effort. This is very useful! As we are using the library's Fortran API, could you also provide a Fortran version of the test file? I had some earlier test, and it seems some source code is still hard-coded with |
|
@matthew-mizielinski Are you okay with these changes? If so, we can have these changes merged. @durack1 Are these change sufficient enough that we can close issue #843 and PR #851? @YanchunHe We'll create another issue for creating a Fortran CMIP7 unit test and have it handled in a separate PR. |
|
@mauzey1 as many of the parts (CVs, tables, etc) are still moving, it might be best to merge these changes, have a nightly created and for testing to continue before a release is minted. @matthew-mizielinski and @JamesAnstey are you ok with this approach? |
|
Hi @mauzey1, I think this looks ok to me. I will note that on second review that you've turned some CMIP6 style tests into CMIP7 style tests rather than adding tests -- this could allow issues with the CMIP6 style functionality to creep in, but the risk of this will be low.
Yes, I don't think we need a release until some of the issues related to repacking have been looked at. |
durack1
left 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.
@mauzey1 apologies for the delay, let's get this merged and into nightly
Resolves #879
CMIP7 unit tests will now use tables from cmip7-cmor-tables, which is now a submodule in this repo.
This will also allow the adding of
mip_eraanddata_specs_versionas global variables if they are defined in the CV even if they are not listed inrequired_global_attributes.@matthew-mizielinski Would you mind reviewing this PR?