Feature #2833 pcp_combine_missing

JohnHalleyGotway commented 4 months ago

Expected Differences

[x] Do these changes introduce new tools, command line arguments, or configuration file options? [Yes]
If yes, please describe:
Adds new -input_thresh command line option for pcp_combine which is supported for the -add, -derive, and -sum options. It's NOT supported for -subtract for which exactly two valid input files are required.
[x] Do these changes modify the structure of existing or add new output data types (e.g. statistic line types or NetCDF variables)? [No]
If yes, please describe:

Pull Request Testing

[x] Describe testing already performed for these changes:
Added 1 new pcp_combine_sum_GRIB1_MISSING test in unit_pcp_combine.xml to demonstrate running -sum with one of the 4 expected inputs missing but -input_thresh 0.75 set to allow output to be written.

Modified 1 existing pcp_combine_derive_MULTIPLE_FIELDS test in unit_pcp_combine.xml to demonstrate running -derive with missing inputs files at the beginning, middle, and end of the input file list. The first 2 do not trigger warnings because the MISSING keyword is used. The 3rd does, because MISSING is not present in the path.

[x] Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
Please review the code changes, documentation updates, and testing changes.
[x] Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]
[x] Do these changes include sufficient testing updates? [Yes]
[x] Will this PR result in changes to the MET test suite? [Yes]
If yes, describe the new output and/or changes to the existing output:
One new output file named pcp_combine/nam_2012040900_F087_APCP12.nc is created.
[x] Will this PR result in changes to existing METplus Use Cases? [No]
If yes, create a new Update Truth METplus issue to describe them.
[x] Do these changes introduce new SonarQube findings? [Yes or No]
If yes, please describe: I don't know. If the GHA run for this PR triggers them, I'll go fix them.
[x] Please complete this pull request review by [Tues 5/14/24].

Pull Request Checklist

See the METplus Workflow for details.

[x] Review the source issue metadata (required labels, projects, and milestone).
[x] Complete the PR definition above.
[x] Ensure the PR title matches the feature or bugfix branch name.
[x] 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 MET-X.Y.Z Development project for official releases
[x] After submitting the PR, select the :gear: 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.

JohnHalleyGotway commented 4 months ago

Converted PR to draft while I fix the tests and SonarQube findings.

JohnHalleyGotway commented 4 months ago

@georgemccabe I just reopened this PR after fixing the testing and SonarQube issues.

Note that 14 SonarQube code smells remain for this PR, but these are ones I do not plan to fix. They're not easy, and I confirmed with Julie and Michelle that these don't have a high enough priority level to be worked on right now.

I do expect differences in the output to be flagged, but only based on the addition of 1 new file.

JohnHalleyGotway commented 4 months ago

Phew, OK, this testing workflow run now completes all the tests and only flags one new output file, as expected:

ERROR: folder /data/output/met_test_truth missing 1 files
    pcp_combine/nam_2012040900_F087_APCP12.nc

The updated logic in parse_file_list.cc is working as expected. This PR is ready to be re-reviewed.

dtcenter / MET