Skip to content

Conversation

hsoh-u
Copy link
Collaborator

@hsoh-u hsoh-u commented Mar 6, 2023

Expected Differences

The 8 digits number should be interpreted as yyyymmdd (converted to unixtime in seconds unit).

  • 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

  • Describe testing already performed for these changes:

This command does not work because $MET_TEST_INPUT (/d1/projects/MET/MET_test_data/unit_test) is not mounted at seneca.

/d1/personal/hsoh/git/pull_request/MET_bugfix_2482_timeslicing_yyyymmdd/bin/plot_data_plane /d1/projects/MET/MET_test_data/unit_test/model_data/nccf/gtg/latlon/gtg_obs_forecast.20130730.i00.f00.nc plot.ps 'name="edr"; level="(20130730,0,*,*)";

Alternative test:

/d1/personal/hsoh/git/pull_request/MET_bugfix_2482_timeslicing_yyyymmdd/bin/plot_data_plane /d1/projects/MET/MET_test_data/unit_test.sav/model_data/nccf/GloTEC_TEC_2015_03_17.nc tmp_test.ps 'name="TEC"; level="(20150317,,)"; file_type=NETCDF_NCCF;' -v 4

Before: "20150317" to 20150317

WARNING: MetNcCFDataFile::convert_time_to_offset() -> 20150317 does not exist at time variable

After: "20150317" to 1426550400

WARNING: MetNcCFDataFile::convert_time_to_offset() -> 1426550400 does not exist at time variable

date +%s -d "20150317" -u
1426550400

Note: $MET_TEST_INPUT (/d1/projects/MET/MET_test_data/unit_test) is not mounted at seneca.

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

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

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

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

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

  • Please complete this pull request review by [Fill in date].

Pull Request Checklist

See the METplus Workflow for details.

  • 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)
    Select: Organization level software support Project or Repository level development cycle Project
    Select: Milestone as the version that will include these changes
  • After submitting the PR, select Development issue 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.

@hsoh-u hsoh-u requested a review from JohnHalleyGotway March 6, 2023 21:52
@JohnHalleyGotway JohnHalleyGotway added this to the MET 11.1.0 milestone Mar 9, 2023
@JohnHalleyGotway JohnHalleyGotway linked an issue Mar 9, 2023 that may be closed by this pull request
21 tasks
@JohnHalleyGotway JohnHalleyGotway changed the title #2482 yyyymmdd format for time slicing Bugfix #2482 develop time slicing yyyymmdd Mar 9, 2023
Copy link
Collaborator

@JohnHalleyGotway JohnHalleyGotway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve of these changes. The change to the logic makes sense, attempting to interpret as timestrings prior to interpreting as numeric indices.

I tested this in seneca:/d1/projects/MET/MET_pull_requests/met-11.1.0/met-11.1.0-beta2/bugfix_2482/MET by running:

bin/plot_data_plane /d1/projects/MET/MET_test_data/unit_test/model_data/nccf/radolan_sp.nc plot.ps name="PREC"; level="(20200101,*,*)"; 

It ran as expected, as did setting that dimension to "0", "2020010100", "20200101_00", and "20200101_000000".

As expected, setting it to bad values failed:
"2": MetNcCFDataFile::convert_time_to_offset() -> 2 does not exist at time variable
"20200101_0000": VarInfoNcCF::set_magic() -> trouble parsing NetCDF dimension value "20200101_0000"!

@hsoh-u hsoh-u merged commit 5c63627 into develop Mar 9, 2023
@hsoh-u hsoh-u deleted the bugfix_2482_timeslicing_yyyymmdd branch March 9, 2023 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Bugfix: Fix support for the YYYYMMDD format in NetCDF level timestrings
2 participants