-
Notifications
You must be signed in to change notification settings - Fork 38
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
feature 780 unsupported MET config overrides #792
Conversation
@georgemccabe looks like this just involves some of the commits on #768 right? So I can continue working as I was planning on that branch? |
@DanielAdriaansen , I tried to cherry pick some of the commits from the branch you are working in so we can get this functionality tested and added to develop since other people need it for their cases. The commits here are slightly different than our branch because ours has additional functionality. Once this PR is merged into develop, there will probably be some minor conflicts to resolve when merging those changes into this branch. I can take care of that part. Yes, you can continue working in the branch. |
…, added new tests to ensure new behavior works correctly
…eck to ensure they are both set has been removed
…most uses of the function will behave as it used to. calling function can include recurse=True when a dictionary item is likely used, i.e. setting MET config values
…s and prevent crashing if improperly formatted value is provided (per #674). Updated tests to ensure proper results
…le in another section like [user_env_vars], added test for that case
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.
New functions here allowed for use case to run that needed MET config override options that are outside the standard currently used. String interpretation works as described, and utility of this kind is strongly needed.
Pull Request Testing
Describe testing already performed for these changes:
Made sure all tests pass
Tested out a few wrappers overriding variables and confirmed the values were successfully overridden
Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
Get use case that needs to override climo field names to work
Do these changes include sufficient documentation and testing updates? [Yes]
I added the new config variables to the glossary and python wrappers pages in the User's Guide. More information on how to use this functionality seems like it would fit in the new sections that @DanielAdriaansen is adding as part of the other work relating to Update setting of environment variables for MET config files to add support for all to METPLUS_ vars #768
Will this PR result in changes to the test suite? [No]
If yes, describe the new output and/or changes to the existing output:
Pull Request Checklist
See the METplus Workflow for details.
Select: Reviewer(s), Project(s), and Milestone