search

Unidata / netcdf-java

The Unidata netcdf-java library
https://docs.unidata.ucar.edu/netcdf-java/current/userguide/index.html
BSD 3-Clause "New" or "Revised" License
142 stars 69 forks source link

cdm-radial: sigmet fixes #1295

Closed michaeldiener closed 5 months ago

michaeldiener commented 7 months ago

Ultimately all these changes add support for IDEAM Colombia files

Addresses the github issue Unidata/netcdf-java#1292

I worked with this version of the Sigmet/IRIS specification: IRIS-Programming-Guide-M211318EN.pdf (not attaching it because of copyright)

PR Checklist

haileyajohnson commented 7 months ago

Hi @michaeldiener , thanks for contributing! I've marked this PR as a draft, please set it to ready for review when it's ready!

michaeldiener commented 7 months ago

Sorry for another commit today, it was a last minute fix. All done now, thank you.

michaeldiener commented 7 months ago

I'm really sorry, I have encountered another issue. Keeping this as a draft now for a few days to avoid wasting anybody's time.

michaeldiener commented 7 months ago

@haileyajohnson The test here seems to fail due to a connection issue when I'm not mistaken. This seems to happen in a module totally unrelated to the one I modified. Is this a known issue? How to proceed?

`dap4.test.TestHyrax > test[5: nc4_strings_comp.nc] STANDARD_ERROR Comparison: vs /github/workspace/dap4/src/test/data/resources/baselinehyrax/nc4_strings_comp.nc.ncdump

21 CHANGED FROM station = { "one", "two", "three", "four", "five", "one_b", "two_b", "three_b", "four_b", "five_b", "one_c", "two_c", "three_c", "four_c", "five_c", "one", "two", "three", "four", "five", "one", "two", "three", "four", "five", "one_f", "two_f", "three_f", "four_f", "five_f" } scan_line = "r", "r1", "r2", "r3", "r4" codec_name = "mp3" lat = {0, 10, 20, 30, 40, 50} lon = {-140, -118, -96, -84, -52} } CHANGED TO dap4.core.util.DapException: Request failure: 500: http://test.opendap.org/opendap/nc4_test_files/nc4_strings_comp.nc.dap?dap4.checksum=true Dap4 Testing: End of differences.

dap4.test.TestHyrax > test[5: nc4_strings_comp.nc] FAILED java.lang.AssertionError: *** FAIL at org.junit.Assert.fail(Assert.java:89) at org.junit.Assert.assertTrue(Assert.java:42) at dap4.test.TestHyrax.test(TestHyrax.java:186)`

haileyajohnson commented 7 months ago

@michaeldiener that looks like a flaky test. I'll rerun your tests and if it fails again I'll turn that one off.

haileyajohnson commented 7 months ago

we're taking a look at the flaky tests but feel free to mark this ready when you want us to review it

michaeldiener commented 7 months ago

@haileyajohnson Thank you! I'm going to wait for a few more days, just to make sure everything keeps running smoothly in my own server environment. Also I'm trying to find more Sigmet IRIS example files, especially ones that contain data types with more than one byte per bin, e.g. DB_DBZ2 Reflectivity (2 byte). This seems harder than anticipated. I sent a support request to Vaisala, they are the ones that create the original Sigmet IRIS software, but not sure they will respond as I'm not one of their customers. Other than that I'm not sure where I can find a file like this.

michaeldiener commented 7 months ago

Found an example of a file I was looking for eventually. All working well now IMHO.

haileyajohnson commented 5 months ago

@michaeldiener I pushed a few changes to your branch and I'm gonna run it against our full test suite, but if that passes I'd say it's ready to go. Thanks again for contributing!

michaeldiener commented 5 months ago

Please let me go through your changes before merging. At first sight there are a few things I want to have a closer look.

michaeldiener commented 5 months ago

@haileyajohnson Thanks for the review! Unfortunately some of your optimizations introduced 2 new bugs and I have resolved this in my latest commit.

haileyajohnson commented 5 months ago

Thanks! We're having some technical difficulties with our test data at the moment, so I'm waiting on that to resolve to run our tests :)