Skip to content

Feature #3218 warn overwrite, #2110 fail if output exists - #3332

Draft
georgemccabe wants to merge 46 commits into
developfrom
feature_3218_warn_overwrite
Draft

Feature #3218 warn overwrite, #2110 fail if output exists#3332
georgemccabe wants to merge 46 commits into
developfrom
feature_3218_warn_overwrite

Conversation

@georgemccabe

@georgemccabe georgemccabe commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Change Summary

  • Added WARN_IF_DUPLICATE_OUTPUT (when the same output is written within the same METplus run) and wrapper-specific config options -- wrt Create WARNING message for operations that overwrite output files #3218
  • Added WARN_IF_OUTPUT_EXISTS (when output already exists on disk) and wrapper-specific config options -- wrt Add config option to fail if output already exists #2110
  • Added EXIT_ON_WARN config option to end METplus run immediately when a warning log is encountered (note: issue Enhancement: Support wrapper-specific EXIT_ON_WARN config options #3333 was created for adding support for wrapper-specific options) -- wrt Add config option to fail if output already exists #2110
  • Modified TCStat wrapper to ignore forecast leads to prevent redundant runs of the tool
  • Downgraded some warnings to debug messages because they are more informative than needing to raise an alert
  • Set EXIT_ON_WARN for all use case tests, requiring that all warnings be resolved or ignored for use cases added to the repository
  • Updates some use case config files to prevent unnecessarily warnings/errors
  • Added tests to demonstrate behavior of new config variables
  • Added documentation to describe how output templates are substituted from different runtime frequency settings, which clarifies confusion discussed in series_analysis in a complex use case #3066
  • Updated pillow package version in requirements.txt files to address security vulnerabilities
  • (Internal) moved pytest markers from pytest.ini file to pyproject.toml to prevent warnings when tests are run from other directories besides internal/tests/pytests
  • (Internal) Enhanced add_met_config_helper.py script to help add config support (documentation, basic use case conf updates, etc.) for all (or multiple) wrappers and for non-MET config variables (skips steps specific to MET tools)

Pull Request Testing

  • Describe testing already performed for these changes:

Added unit tests to demonstrate expected behavior

  • Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:

John HG - review code changes under metplus directory and unit tests under internal/tests/pytests to confirm correctness. Review use case diffs (see below)

? - review documentation changes - Are the new configs named well? Should there be a new section in the User's Guide (under Configuration chapter?) to describe warning, config variables to skip certain warnings, and exiting when a warning is encountered?

  • Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]

Unless review suggests adding more

  • Do these changes include sufficient testing updates? [Yes]

  • Will this PR result in changes to the test suite? [Yes]

    If yes, describe the new output and/or changes to the existing output:

met_tool_wrapper:30-58,68-69

  • UserScript_run_once - different number of files found due to init times skipped to prevent warnings
  • GenEnsProd - different number of output files used/expected to prevent warnings

medium_range:3-5,10

  • medium_range/GridStat_fcstGFS_obsGFS_climoNCEP_MultiField -- more output files that contain field name/level in filename because previously the same file was being overwritten multiple times

s2s_mid_lat
FAILED DUE TO WARNING FOR EXPECTED MISSING FILE (in create_cbl_filelist but maybe more?) -- need to think of a solution...

  • Do these changes introduce new SonarQube findings? [No]

    If yes, please describe:

  • Please complete this pull request review by 2026/8/18.

Pull Request Checklist

See the METplus Workflow for details.

  • Add any new Python packages to the METplus Components Python Requirements table.
  • For any new datasets, an entry to the METplus Verification Datasets Guide.
  • Review the source issue metadata (required labels, projects, and milestone).
  • 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) and Development issue
    Select: Milestone as the version that will include these changes
    Select: Coordinated METplus-X.Y Support project for bugfix releases or METplus-Wrappers-X.Y.Z Development project for official releases
  • After submitting the PR, select the ⚙️ icon in the Development section of the right hand sidebar. Search for the issue that this PR will close and select it, if it is not already selected.
  • 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.

…ing overwritten, indicating that a misconfiguration is present. Pass list of outputs to the next wrapper in the process list so it can properly check if multiple instances are writing to the same path. Move logic to run processes into a function that can be called by unit tests to test behavior from running multiple processes, e.g. multiple instances of the same wrapper.
…t a duplicate file path is being overwritten
…sting various scenarios such as: 2 fields processed separately, runtime freq doesn't match output template, custom loop list, and multiple instances. This includes updated test logic to properly test multiple PROCESS_LIST items instead of creating/calling a single wrapper/process as many test do. Consider adding a fixture to easily handle the config and process list initialization that can be resused in many tests
…m templates for each of the RUNTIME_FREQ settings
…sues by changing the output template and/or prefix
…multiple wrappers at once and 2) skip steps specific to MET config variables
…dd to warning message with info on how to disable it
…nings when output is written more than once in a single METplus run
… encountered. This setting coupled with the new logic to warn if output has already been generated within a given METplus run will cause a run to fail in this case. Note that this does not warn/fail if the output already exists on disk from a previous run
…ee which use cases currently output a warning log
…lid behavior to read skip lead seq setting and skip if set
…me valid is processed more than once based on the configuration and it doesn't need to be run multiple times
…s when a warning occurs and EXIT_ON_WARN=True
…cause the exception will prevent the info log from being output
… output a warning when the output file attempted to be written already exists on disk. Coupled with EXIT_ON_WARN, this accomplishes the desired behavior in issue #2110
…init and running processes. Note currently EXIT_ON_WARN is only supported on the global level and is read upon initial METplusConfig initialization, so the test had to be written to set this config setting before initializating the config.
… a warning log is still output if SKIP_IF_OUTPUT_EXISTS is not set
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 30297088544

Coverage increased (+0.1%) to 92.184%

Details

  • Coverage increased (+0.1%) from the base build.
  • Patch coverage: 5 uncovered changes across 3 files (91 of 96 lines covered, 94.79%).
  • 1 coverage regression across 1 file.

Uncovered Changes

File Changed Covered %
metplus/wrappers/command_builder.py 30 28 93.33%
metplus/wrappers/runtime_freq_wrapper.py 6 4 66.67%
metplus/wrappers/point_stat_wrapper.py 1 0 0.0%
Total (17 files) 96 91 94.79%

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File Lines Losing Coverage Coverage
metplus/wrappers/runtime_freq_wrapper.py 1 91.03%

Coverage Stats

Coverage Status
Relevant Lines: 10530
Covered Lines: 9707
Line Coverage: 92.18%
Coverage Strength: 1.84 hits per line

💛 - Coveralls

@georgemccabe georgemccabe changed the title Feature #3218 warn overwrite, #2100 fail if output exists Feature #3218 warn overwrite, #2110 fail if output exists Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🔎 In review

Development

Successfully merging this pull request may close these issues.

Create WARNING message for operations that overwrite output files Add config option to fail if output already exists

2 participants