-
Notifications
You must be signed in to change notification settings - Fork 64
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
Restore capability to export metadata sections (previously headers) to html #445
Restore capability to export metadata sections (previously headers) to html #445
Conversation
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.
A couple of suggestions but basically okay.
scripts/metadata_table.py
Outdated
@@ -1266,6 +1279,36 @@ def is_scalar_reference(test_val): | |||
"""Return True iff <test_val> refers to a Fortran scalar.""" | |||
return check_fortran_ref(test_val, None, False) is not None | |||
|
|||
def to_html(self, outdir, props): |
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.
I would move this up to where the other active routines are, say right after write_to_file
.
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.
Done.
scripts/metadata_table.py
Outdated
# Write table header | ||
header = "<tr>" | ||
for prop in props: | ||
header += "<th>{}</th>".format(prop) |
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.
I have been converting to f strings, this seems like a good place.
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.
I have adopted f strings in this PR, thanks for the suggestion.
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.
Tested. It works well.
@mkavulich @grantfirl I addressed @gold2718's review comments, now it is your turn ... |
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 OK to me. I did not test myself, but others have reported success, so approved.
@climbfuji Could we combine this with #443 for the purposes of merging with UFS? |
Restore capability to export metadata sections (previously headers) to html.
The capability to write metadata sections to html was removed with the refactoring of the
feature/capgen
branch, subsequently slipped the reviews and got merged into main. As a result, themetadata2html.py
script, required to generate scientific documenation (especially at times of releases!) doesn't work anymore. This PR restores the capability by putting the original code blocks back into the metadata sections (previously these were called headers).I ran this in the two supported ways within the UFS. Once merged, the code will work like this. Maybe there are some slight path changes compared to what is in the CCPP tech doc - @mkavulich FYI.
Batch mode using
ccpp_prebuild_config.py
:Single-file mode:
User interface changes?: Yes (I assume some of the paths have changed compared to the documentation)
Closes #444
Testing: see above. The code changes will also be tested within the ufs-weather-model regression testing environment. I recommend that we pull this PR into #443, since #443 and associated PRs do not change the baselines. Note that #444 is required for CCPP release v6.
Other testing:
ccpp_prebuild.py
:capgen.py
:test removed: none
unit tests: none
system tests: none
manual testing: see above