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

Fix/report Python3 decoding errors, parse all suites if --suites=... argument not present #292

Merged
merged 1 commit into from
May 12, 2020

Conversation

climbfuji
Copy link
Collaborator

@climbfuji climbfuji commented May 11, 2020

This PR:

Associated PRs:
NOAA-EMC/GFDL_atmos_cubed_sphere#20
#292
NCAR/ccpp-physics#451
NOAA-EMC/fv3atm#115
NOAA-EMC/NEMS#62
ufs-community/ufs-weather-model#126

For regression testing information, see ufs-community/ufs-weather-model#126.

… decoding non-ascii files in scripts/metadata_parser.py, parse all suite definition files if suites argument is not present when calling ccpp_prebuild.py
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.

Not sure about some of the design decisions but seems okay.

Comment on lines +164 to +176
def get_all_suites(suites_dir):
success = False
logging.info("No suites were given, compiling a list of all suites")
sdfs = []
for f in os.listdir(suites_dir):
match = SUITE_DEFINITION_FILENAME_PATTERN.match(f)
if match:
logging.info('Adding suite definition file {}'.format(f))
sdfs.append(f)
if sdfs:
success = True
return (success, sdfs)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a lot of work to do instead of something like:

sdfs = glob.glob(suite_dir, "suite_*.xml")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right ... when I was making those changes I wasn't sure whether I needed to get the suite name (which comes for free as match.group(1) with my approach) or not. Turned out I didn't.

In any case, even the more expensive approach costs probably less than a tenth of a second, so that should be ok.

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 good to me. Approved.

Copy link
Contributor

@llpcarson llpcarson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved.

@climbfuji climbfuji merged commit 7c3673d into NCAR:master May 12, 2020
@climbfuji
Copy link
Collaborator Author

@JulieSchramm @ligiabernardet the changes in this PR require an update of the technical documentation ... looking for volunteers ...

@ligiabernardet
Copy link
Collaborator

ligiabernardet commented May 21, 2020 via email

@climbfuji climbfuji deleted the python3_decoding_parse_all_suites branch June 27, 2022 03:05
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.

5 participants