-
Notifications
You must be signed in to change notification settings - Fork 49
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
dtc/develop: Python 3 compatibility (run scripts), standard_name updates #179
dtc/develop: Python 3 compatibility (run scripts), standard_name updates #179
Conversation
…ters for ccpp-framework and ccpp-physics
…e standard names following the radiation tendency cleanup work
…atibility, generate report at end of multi-run script when verbose flag is set
…y.SourceFileLoader throw different exceptions
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 good to me as well. There weren't too many changes for python 3 in the run scripts it looks like, but it saved me some time figuring it out, so thanks! Approved.
@grantfirl do you want to test it first? I am happy to accommodate any changes you may have as the primary gmtb-scm developer. |
I cloned this recursively (I have uncommitted changes in my working version), built and ran on my machine in python 2.7 and 3.6 environments. Everything worked as expected, including the new summary in verbose mode. The only thing that I noticed is that the verbose output in the multirun script using 3.6 was not parsing newline characters in the output correctly, so it looked terrible, but that isn't a huge deal and I can investigate that later. |
Yes, I noticed that, too. Must have to do with the Python 3.x way of treating strings as bytes. Maybe adding .decode() or something like that? For the future ... will merge now. |
…tendencies dtc/develop: Python 3 compatibility (run scripts), standard_name updates
This PR:
scm/src/run_gmtb_scm.py
andscm/src/multi_run_gmtb_scm.py
This PR requires NCAR/ccpp-physics#426 to be merged first.
Tested on macOS with