-
Notifications
You must be signed in to change notification settings - Fork 24
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 #2870 removing_MISSING_warning #2872
Conversation
…file list and for logging missing files, checking for the MISSING keyword. Also, update Ensemble-Stat and Gen-Ens-Prod to call these functions.
…e MISSING keyword for missing files. METplus uses this keyword for Ensemble-Stat and Gen-Ens-Prod.
Note that all of the differences flagged in this testing workflow run are unrelated to these changes. They are remaining diffs from @hsoh-u recent PR that modified the UGRID output. We need to update
|
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.
These changes look good. These changes will play nicely with the METplus wrappers logic that handles missing files, which either sets MISSING
or MISSING/some/path
instead of a file path. The tests look to demonstrate these cases. The new output looks good. I don't think it is necessary to update the documentation to describe this change. I approve.
This PR includes the following changes:
parse_file_list_type(const StringArray&)
utility function to loop through the input files and determine the file type of the first non-missing file.log_missing_file(...)
utility function to write a warning message about missing files or just DEBUG(3) if the missing file name begins with theMISSING
keyword from METplus wrappers.Note the following two tweaks to the logic:
Calling
parse_file_list_type(...)
instead enables the first file in the list to be missing. So that's an improvement.open()
on the first input file, and that may have applied a little extra validation logic. But that isn't REALLY needed here because we open the files later when we process them. Not opening them here is every so slightly more efficient but it's possible that if a runtime error was going to occur, it may have occurred sooner rather than later in the logic.Expected Differences
Do these changes introduce new tools, command line arguments, or configuration file options? [No]
If yes, please describe:
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
Observe the following log messages... 2 DEBUG and 1 WARNING:
Observe the following log messages...
I'll note that providing a
-grid_obs
bad path as the first one in the list (-grid_obs /a/bath/path
) does result in a runtime error. So this PR doesn't solve all the issues with bad data inputs... just the ones described in the issue.Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
The code for this PR is compiled and available for testing as the
met_test
user inseneca:/d1/projects/MET/MET_pull_requests/met-12.0.0/beta5/MET-feature_2870_remove_MISSING_warning
.Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [No]
I couldn't decide whether or not documenting the subtle difference between a warning and debug log message for missing files that start with the
MISSING
keyword is really warranted. So I did NOT add a note about to the user's guide. Do you think I should?Do these changes include sufficient testing updates? [Yes]
I updated the existing
unit_gen_ens_prod.xml
file. It includes 3 references to agep3
file that does NOT actually exist. I increased the verbosity level to 3 for all these runs and updated those 3 references to:MISSING
MISSING/&DATA_DIR_MODEL;/grib1/arw-tom-gep3/arw-tom-gep3_2012040912_F024.grib
&DATA_DIR_MODEL;/grib1/arw-tom-gep3/arw-tom-gep3_2012040912_F024.grib
The first 2 should produce a DEBUG(3) log message and the 3rd should still produce a warning message.
Will this PR result in changes to the MET test suite? [No]
If yes, describe the new output and/or changes to the existing output:
Will this PR result in changes to existing METplus Use Cases? [No]
If yes, create a new Update Truth METplus issue to describe them.
I will change the log output from METplus ensemble use cases, but not the actual output.
Do these changes introduce new SonarQube findings? [No]
If yes, please describe:
This PR passed the SonarQube quality gate check.
Please complete this pull request review by [Fri 5/3/24].
Pull Request Checklist
See the METplus Workflow for details.
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