Remove optional arguments from CCPP (capgen metadata parser and ccpp_prebuild scripts)#408
Conversation
…d ccpp_prebuild scripts
…move_optional_arguments_from_ccpp
|
BTW, metavar.py is one of the files I have refactored so I will have to wait for this PR to be merged and then brought into feature/capgen before I can open my PR. |
We could merge it right away into feature/capgen and then later - exactly the same commit history - into main, then there will be no issues later on. Or you create your PR and I resolve the merge conflict later. I prefer option 1. |
If you want to do that, you will need to close this PR and open a new one. It is okay with me to go that way. We still need another review. |
We can keep this one open and create a second one to feature/capgen. As long as we don't change the commits in one of them, we'll be fine. Closing this PR is too much work (PR numbers cross-referenced in all the other PRs etc). |
grantfirl
left a comment
There was a problem hiding this comment.
Looks straightforward, thanks.
This is a bit of a risk. Do you need the metavar.py change for prebuild? If not, I can add that change to my PR. |
|
Yes, I do to prevent "optional" attributes being reintroduced. There is nothing risky about this process, we do this all the time. But anyway, let's proceed with your update of feature/capgen and I will resolve the merge conflict afterwards.
… On Oct 26, 2021, at 12:43 PM, goldy ***@***.***> wrote:
As long as we don't change the commits in one of them, we'll be fine.
This is a bit of a risk. Do you need the metavar.py change for prebuild? If not, I can add that change to my PR.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#408 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5C2RJOZ2H7YSWYP6V75YLUI3ZG5ANCNFSM5GVJHGRA>.
Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Okay then, proceed. I will add my changes to yours once they get to feature/capgen |
Description
Remove support for optional arguments from
capgen's metadata parser. The code now raises an Exception when it finds anoptionalattribute. Note that thecapgenmetadata parser is used byccpp_prebuild.Remove support for optional arguments from
ccpp_prebuildand all its helper scripts.Fix two failing doctests in
metavar.pyandparse_checkers.py.After this PR, the feature/capgen branch must be updated from main.
User interface changes?
Yes. Need to remove all optional attributes from the metadata.
Note also that the
OPTIONAL_ARGUMENTSsection in the CCPP prebuild config is ignored.Issues
Fixes #407
Associated PRs
#408
earth-system-radiation/rte-rrtmgp#143
NCAR/ccpp-physics#766
NOAA-EMC/ufsatm#416
ufs-community/ufs-weather-model#892
ESCOMP/ESMStandardNames#23
For regression testing with the UFS, see ufs-community/ufs-weather-model#892.