Conversation
|
Would you like to contribute one of your minimalist grib files for testing? We don't need to actually load the >200MB of coordinate grid, just verify that we can generate the one reference. |
|
It may take some time. I believe the file we have is technically proprietary, but I'll try to generate a very small GRIB file for testing. |
|
OK; I don't think the change here is contraversions, so happy to accept it without too. |
|
I cannot find a public GRIB file that is small for this test. I'm sorry. And I can't seem to figure out how to create one that is very small. (Have no idea how this one was created!) |
|
OK :) For the failing test, can you see if this is passing for you locally? Was there a new version of cfgrib or something? |
|
I can reproduce the error locally. At the moment, I'm not sure the cause. Seems unrelated to the changes I made in this PR, but I'll see if I can track it down. |
|
This test failed when I run in the |
|
The failing test also has nothing to do with GRIB. So, it's probably not a |
|
It is failing with the latest version of Xarray: 2024.3.0. If you downgrade to 2024.2.0, the tests pass. |
Bahh. I wonder if then the assert is too strict. It might, then, have something to do with pandas version too, since their time types changed within the last year. |
|
I can verify that if you comment out the |
|
I can make that change in the test if that's acceptable. |
|
perhaps |
|
Done! |
kerchunk/tests/test_combine.py
Outdated
| engine="zarr", | ||
| chunks={}, | ||
| ) | ||
| assert z.time.dtype == "M8[s]" |
There was a problem hiding this comment.
Not sure what coo_dtypes does, but Xarray definitely normalizes everything to ns. Perhaps a bug was inadvertently fixed.
There was a problem hiding this comment.
Yes. I'm not sure what coo_dtypes does, either, but it seems to no longer be doing what is expected with the latest version of Xarray.
|
I will have to return to this later (maybe tomorrow)! |
|
I'm back to this now, and I have figured out how to manufacture a special tiny GRIB2 file for testing, with nothing proprietary in it. So, I've added a test for this PR. There are still failing tests which have to do to updates/changes in other dependencies that will be dealt with in PR #450. So, I'll get back to this. |
|
If #450 is merged, I believe that all of the tests (including the newly added one for this PR) should pass or be skipped as expected. |
|
You'll need to merge from main I think |
|
Yay! |
3 participants
This addresses issue #448.
closes #448