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

[5.5.3]: Fix GRIB IOSP time coords from intervals, for 2-D time datasets #1248

Conversation

ethanrd
Copy link
Member

@ethanrd ethanrd commented Oct 13, 2023

Description of Changes

Fix generation of time coordinate variables from time intervals for 2-D time datasets. Ensure resulting time coordinate variables are strictly monotonically increasing. Handle when time intervals are irregular and overlapping.

See issue #1060.

PR Checklist

  • Link to any issues that the PR addresses
  • Add labels
  • Open as a draft PR
    until ready for review
  • Make sure GitHub tests pass
  • Mark PR as "Ready for Review"

@ethanrd ethanrd requested a review from tdrwenski October 13, 2023 18:09
@ethanrd ethanrd added the iosp: grib grib file format label Oct 13, 2023
@ethanrd ethanrd marked this pull request as ready for review October 19, 2023 17:54
@tdrwenski tdrwenski force-pushed the fix_2D_time_coord_var_from_time_intervals-v2 branch from a981823 to 0a0493f Compare October 19, 2023 19:26
Copy link
Contributor

@tdrwenski tdrwenski left a comment

Choose a reason for hiding this comment

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

I ran all the tests on Jenkins against your branch and one is failing: https://jenkins-aws.unidata.ucar.edu/view/Users/job/tara-netcdf-java/35/

In the TestGribCompareBuilders tests it looks like it is comparing the builder/ non-builder versions for all files in ../grib/src/test/data/ and it looks like there's a difference for the new file you added. I do not believe this is actually related to your changes (I ran the test with that file on maint-5.x and it also failed). Since it's unrelated and I'm not sure how important it is that the with/without builders return identical results, maybe it's easiest to just ignore that new test file in that test and possibly fix it later.

@ethanrd
Copy link
Member Author

ethanrd commented Oct 20, 2023

Agree, probably easier to ignore the new test file. Are there other test files excluded from this test?

@tdrwenski
Copy link
Contributor

Agree, probably easier to ignore the new test file. Are there other test files excluded from this test?

Just pushed a fix to ignore that test case!

@tdrwenski tdrwenski self-requested a review October 20, 2023 20:11
@tdrwenski tdrwenski merged commit 925d10b into Unidata:maint-5.x Oct 20, 2023
9 checks passed
@ethanrd ethanrd deleted the fix_2D_time_coord_var_from_time_intervals-v2 branch October 30, 2023 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iosp: grib grib file format
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants