Skip to content
This repository has been archived by the owner on Oct 24, 2024. It is now read-only.

implement open_datatree context manager #114

Closed
wants to merge 6 commits into from

Conversation

malmans2
Copy link
Member

I haven't added any test, but the io tests now use the context manager. Not sure if there's a better way to test this functionality.

@TomNicholas TomNicholas added the IO Representation of particular file formats as trees label Jun 17, 2022
@TomNicholas
Copy link
Member

do not look at parent

Did you get a RecursionError when you tried to close the parent too? I've done that before 😅

This seems good though. For testing xarray must have some tests for this type of opening that we could copy.

I'm trying to think if there are any other gotchas with tree structures... .close() closing that node and all children seems fine, though I would like to be clear about what happens if you refer to a closed child via its parent:

subtree = open_datatree(file)
dt = DataTree(children={"folder1": subtree})

dt["folder1"].close()
print(dt)

@TomNicholas TomNicholas added the enhancement New feature or request label Jun 17, 2022
@malmans2
Copy link
Member Author

Did you get a RecursionError when you tried to close the parent too? I've done that before 😅

Yes, which made me realise that I don't think closing parents is the right thing to do. I.e., to close everything users must close from root.

This seems good though. For testing xarray must have some tests for this type of opening that we could copy.

That's what I thought too, but I couldn't find anything. I'll take a better look next week, but let me know if you know where I can find those tests.

I'm trying to think if there are any other gotchas with tree structures... .close() closing that node and all children seems fine, though I would like to be clear about what happens if you refer to a closed child via its parent:

Good point - I'll think about this as well!

datatree/datatree.py Outdated Show resolved Hide resolved
@malmans2
Copy link
Member Author

@TomNicholas I've added open=True/False to the repr. Not sure if this is the best way to go though. What do you think?

@TomNicholas
Copy link
Member

I've added open=True/False to the repr. Not sure if this is the best way to go though. What do you think?

Hmmm... that seems a bit janky to me, especially as Dataset doesn't do that.

I remember talking about this problem briefly with @jhamman - perhaps he has some suggestions as to how to handle multiple file handles?

@TomNicholas
Copy link
Member

Closing in favour of linked upstream issue

@TomNicholas TomNicholas closed this Oct 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request IO Representation of particular file formats as trees
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically close files using open_datatree context manager
2 participants