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

Fix determination of which variables are coordinates #224

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

TomNicholas
Copy link
Member

This is meant to supercede #191 as a general solution to the problem of (e.g. #189).

I do think though that in order to get this test to fully pass (i.e. identify all the coordinates according to the CF conventions, then we need to use xr.decode_cf as described in #157 (comment)).

However, I'm currently having weird issues where my test is not behaving deterministically when running locally. The set of coords identified is not the same on every run!

@TomNicholas
Copy link
Member Author

TomNicholas commented Aug 23, 2024

I do think though that in order to get this test to fully pass (i.e. identify all the coordinates according to the CF conventions, then we need to use xr.decode_cf as described in #157 (comment)).

I've now started doing this, and the general approach seems promising (cc @sharkinsspatial).

I have 2 issues to solve before I can properly bug-hunt:

  1. I had to comment out this line to avoid wrapping the ManifestArray with xarray's lazy indexing classes. I think that perhaps xarray should be changed upstream here - if I don't decode any values at all (and only alter metadata) then there is no need to wrap it in a lazy indexing class, and wrapping everything at this point seems like a mixing of different concerns inside one function. (@dcherian wondering what you think though)
  2. Somehow my code is now getting to here in xarray, which tries to create a 1D pandas index, which attempts to load a ManifestArray into memory, which errors. I thought I had found all the ways that could happen so this is annoying, because (a) it might require another change to xarray upstream and (b) I do not understand why this is happening with this PR but not without this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

open_virtual_dataset returns some coordinates as data variables
1 participant