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

Remove optional arguments from CCPP (capgen metadata parser and ccpp_prebuild scripts) #408

Merged
merged 2 commits into from
Nov 3, 2021

Conversation

climbfuji
Copy link
Collaborator

@climbfuji climbfuji commented Oct 25, 2021

Description

  1. Remove support for optional arguments from capgen's metadata parser. The code now raises an Exception when it finds an optional attribute. Note that the capgen metadata parser is used by ccpp_prebuild.

  2. Remove support for optional arguments from ccpp_prebuild and all its helper scripts.

  3. Fix two failing doctests in metavar.py and parse_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_ARGUMENTS section 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/fv3atm#416
ufs-community/ufs-weather-model#892
ESCOMP/CCPPStandardNames#23

For regression testing with the UFS, see ufs-community/ufs-weather-model#892.

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, thanks!

@gold2718
Copy link
Collaborator

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.

@climbfuji
Copy link
Collaborator Author

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.

@gold2718
Copy link
Collaborator

We could merge it right away into feature/capgen and then later

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.

@climbfuji
Copy link
Collaborator Author

We could merge it right away into feature/capgen and then later

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).

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.

Looks straightforward, thanks.

@gold2718
Copy link
Collaborator

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.

@climbfuji
Copy link
Collaborator Author

climbfuji commented Oct 26, 2021 via email

@gold2718
Copy link
Collaborator

There is nothing risky about this process, we do this all the time.

Okay then, proceed. I will add my changes to yours once they get to feature/capgen

@gold2718 gold2718 mentioned this pull request Oct 27, 2021
@climbfuji climbfuji added capgen-unification ccpp_prebuild bugs, requests, etc. that involve ccpp_prebuild labels Oct 27, 2021
@climbfuji climbfuji merged commit 47a5f65 into NCAR:main Nov 3, 2021
@climbfuji climbfuji deleted the remove_optional_arguments_from_ccpp branch June 27, 2022 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
capgen-unification ccpp_prebuild bugs, requests, etc. that involve ccpp_prebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove optional keyword from CCPP metadata (8 hours)
3 participants