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

DataTree.coords.__setitem__ is broken #9204

Closed
shoyer opened this issue Jul 1, 2024 · 1 comment · Fixed by #9451
Closed

DataTree.coords.__setitem__ is broken #9204

shoyer opened this issue Jul 1, 2024 · 1 comment · Fixed by #9451
Assignees
Labels
bug topic-DataTree Related to the implementation of a DataTree class

Comments

@shoyer
Copy link
Member

shoyer commented Jul 1, 2024

One messy thing is that it appears that assignment via .coords is broken on DataTree even at main.

Ah yes - I forgot there's a TODO deep somewhere for that 😅 I left it for later because it seemed like it might require changing the DatasetCoordinates class that ds.coords returns. (Surely that class could just be replaced by the new xr.Coordinates class now??)

Originally posted by @TomNicholas in #9063 (comment)

@TomNicholas
Copy link
Member

I'm trying to have a go at this but it's made harder by #9203 not being implemented. The problem is that the DatasetCoordinates class works by storing the Dataset object as ._data, then altering it in-place by altering ._data._variables and ._data._coord_names. But the same approach doesn't work for DataTree because that uses ._data_variables and ._coord_variables internally, so you can't get away with just passing the DataTree object to the DatasetCoordinates constructor, even though conceptually that should work.

So to make it work right now requires a new DataTreeCoordinates object. That's fine, but if #9203 were completed then we wouldn't really need that new class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug topic-DataTree Related to the implementation of a DataTree class
Projects
Development

Successfully merging a pull request may close this issue.

2 participants