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 legacy src/ccpp_api.F90 - contains #445 (fix metadata2html.py) and #447 (remove NEMSfv3gfs test scripts) #443

Merged
merged 6 commits into from
Apr 12, 2022

Conversation

climbfuji
Copy link
Collaborator

@climbfuji climbfuji commented Mar 20, 2022

Remove legacy src/ccpp_api.F90.

Note. This PR now (04/10/2022) also contains the changes from #445 and #447 (all approved).

User interface changes? Yes

Models currently importing ccpp_t from ccpp_api must get it from ccpp_types directly.

Fixes #442

Testing:

I ran the two tests that Julie set up a while back for ccpp_prebuild.py in directory tests, both of them passed:

> PYTHONPATH=/Users/dom.heinzeller/scratch/ufs-weather-model/ufs-weather-model-split-gfs-typedefs/FV3/ccpp/framework/scripts/parse_tools:/Users/dom.heinzeller/scratch/ufs-weather-model/ufs-weather-model-split-gfs-typedefs/FV3/ccpp/framework/scripts:$PYTHONPATH python3 test_metadata_parser.py
> PYTHONPATH=/Users/dom.heinzeller/scratch/ufs-weather-model/ufs-weather-model-split-gfs-typedefs/FV3/ccpp/framework/scripts/parse_tools:/Users/dom.heinzeller/scratch/ufs-weather-model/ufs-weather-model-split-gfs-typedefs/FV3/ccpp/framework/scripts:$PYTHONPATH python3 test_mkstatic.py

The change is also being tested by the ufs-weather-model regression testing system, see ufs-community/ufs-weather-model#1130

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.

Whew! Finally made it through this review.
Looks okay :)

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.

This looks fine to me. Thanks for pointing out in the description that SCM will need to import ccpp_t from ccpp_types now. I'll make that change in an upcoming PR.

@climbfuji climbfuji changed the title Remove legacy src/ccpp_api.F90 Remove legacy src/ccpp_api.F90 - contains #445 (fix metadata2html.py) and #447 (remove NEMSfv3gfs test scripts) Apr 11, 2022
@climbfuji climbfuji merged commit e200083 into NCAR:main Apr 12, 2022
@climbfuji climbfuji deleted the feature/remove_ccpp_api branch June 27, 2022 02:47
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.

[ccpp_prebuild.py] Cleanup: remove legacy ccpp_api.F90
3 participants