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

feature 780 unsupported MET config overrides #792

Merged
merged 16 commits into from
Feb 18, 2021

Conversation

georgemccabe
Copy link
Collaborator

@georgemccabe georgemccabe commented Feb 4, 2021

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.

  • Complete the PR definition above.
  • Ensure the PR title matches the feature or bugfix branch name.
  • Define the PR metadata, as permissions allow.
    Select: Reviewer(s), Project(s), and Milestone
  • After submitting the PR, select Linked Issues with the original issue number.
  • After the PR is approved, merge your changes. If permissions do not allow this, request that the reviewer do the merge.
  • Close the linked issue and delete your feature or bugfix branch from GitHub.

@georgemccabe georgemccabe added this to the METplus-4.0 milestone Feb 4, 2021
@georgemccabe georgemccabe requested a review from j-opatz February 4, 2021 21:30
@georgemccabe georgemccabe changed the title feature 768 unsupported MET config overrides feature 780 unsupported MET config overrides Feb 4, 2021
@DanielAdriaansen
Copy link
Contributor

@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?

@georgemccabe
Copy link
Collaborator Author

georgemccabe commented Feb 5, 2021

@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
Copy link
Contributor

@j-opatz j-opatz left a 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.

@georgemccabe georgemccabe merged commit 90aa20a into develop Feb 18, 2021
@georgemccabe georgemccabe deleted the feature_768_overrides branch February 18, 2021 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants