-
Notifications
You must be signed in to change notification settings - Fork 36
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
Values loaded by NCDatasets backend with "_Unsigned" CF attribute don't correspond to Xarray #735
Comments
Those just look like different colour schemes? (Otherwise, to be clear on main NCDatasets.jl is still handling CF conversions, not Rasters) The first one should call |
No the color scheme is the same, the color limits are different though and it seems the numbers are different as well. |
Plus you can see the structure is totally strange before |
Yes read is clearly a problem. The dispatch is wrong somewhere it looks like broken iteration in DiskArrays. But the non lazy plots without noise are just NCDatasets.jl vs XArray, Rasters is not affecting the values |
aha ok, will check out what's going on there. This is on the cf branch so hopefully it's not ignoring some obscure CF convention... |
Makes way more sense this is on It most likely is ignoring or getting some CF thing wrong |
FWIW I get this on main too |
Both problems or just one. Like if it's both Rasters and NCDatasets CF outputs are the same on main and cf, then maybe it's Xarray... Does it even do CF scaling by default I think cf_xarray is a separate python package, its not in xarray proper? |
The DiskArrays munging I will look at tonight, seems bad if that's on main |
This comment was marked as outdated.
This comment was marked as outdated.
You don't for this, it's public access. |
This comment was marked as outdated.
This comment was marked as outdated.
Huh! Here's a google drive link: https://drive.google.com/file/d/1Om08vXhgsmLbcGmbEFaachfBHa6Nq5pA/view?usp=sharing |
I can't reproduce your garbled up lazy raster plotting. |
Which OS/DiskArrays versions are you on? I'm on Mac M1 and DiskArrays 0.4.4 |
I am on linux and DiskArrays 0.3.23 |
Ah, might be a DiskArrays 0.4.x thing then? |
How did you manage to get DiskArrays 0.4 are you on the PR branch from Fabian? |
This might be a NCDatasets and DiskArrays 0.4 combination thing, because on the other backends DiskArrays 0.4 didn't produce such problematic plots. |
Ah possibly, yeah I am on Fabian's PR branch for NCDatasets. This also produces the same issues with strange values with the same data but encoded in Zarr by the source (via Kerchunk). So I don't think the values are an interaction issue unless it's an issue with DiskArrays v0.4 in general. The |
There's clearly some structure here that I bet is described by some CF attribute. But I don't know what it is! |
Haha we have every issue with every problem in branches of DiskArrays/CommonDataModel showing up here! Can we keep issues here to registered versions of all deps? I will go insane otherwise |
I can reproduce on a clean env with registerd versions the values that Anshul reported above for opening the data with Raster but I haven't compared to xarray locally. So this issue should rather be about the not applied CF conventions and not about the read, because this might be a DiskArrays 0.4 issue. |
Does Rasters check the |
On main Rasters checks nothing (It's CommonDataModel), in |
It turns out there's an attribute Turns out that was the issue. Might be worth a CommonDataModel.jl issue. But this means that all fillvalues etc need this horribly complicated reinterpretation to unsigned values too. This sounds like a lot of work...but it's a major correctness bug especially on old datasets |
Absolutely worth a CommonDataModel.jl bug report Probably actually NCDatasets.jl. Rasters should not know about types issues like that the variable should be properly typed |
The DiskArrays portion of this issue (striping in the plots) was fixed by JuliaIO/DiskArrays.jl#198. The unsigned attribute likely needs to be handled in NCDatasets.jl |
MWE:
None of this agrees with what xarray loads:
but it looks like all CF conventions are applied...
The text was updated successfully, but these errors were encountered: