-
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
Fix decoding errors in scripts/common.py's execute_cmd #295
Conversation
Note: I flagged this PR and associated PRs as "ready to merge" in case these make it to the top of the commit queue during my two days of holidays. Please do not merge unless agreed upon with @DusanJovic-NOAA and @junwang-noaa. |
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.
Style comment / suggestion.
scripts/common.py
Outdated
message += ' stdout: "{0}"\n'.format(stdout.decode('ascii', 'ignore').rstrip('\n')) | ||
message += ' stderr: "{0}"'.format(stderr.decode('ascii', 'ignore').rstrip('\n')) |
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.
Normally, I would use argument names for optional arguments:
stdout.decode(encoding='ascii', errors='ignore')
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 added the argument names as requested, thanks for the suggestion.
Fix decoding errors in scripts/common.py's execute_cmd
This PR:
Associated PRs:
#295
NOAA-EMC/fv3atm#117
NOAA-EMC/NEMS#63
ufs-community/ufs-weather-model#139
For regression testing information, see below ufs-community/ufs-weather-model#139.