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

BUG: Close file handle with lock=False #346

Merged
merged 1 commit into from
May 24, 2021
Merged

Conversation

snowman2
Copy link
Member

@snowman2 snowman2 commented May 24, 2021

  • Tests added
  • Fully documented, including docs/history.rst for all changes and docs/rioxarray.rst for new API

For the caching file manager, it closes the file handle from the cache ref. Since there is no cache to close with, I am thinking that the class should keep track of it like the DummyFileManager ref. (ping @TomAugspurger in case you have thoughts on this).

@snowman2 snowman2 requested a review from justingruca May 24, 2021 14:14
@TomAugspurger
Copy link
Contributor

I haven't looked closely at the issue / PR, but just want to note that you may want to be careful to only close the file handle when rioxarray is the one that created it. If you're passed an open file handle then typically the user should be the one to close it. Though like I said, I haven't looked closely so I don't know if that applies here.

@snowman2
Copy link
Member Author

Thanks for your comment @TomAugspurger 👍

you may want to be careful to only close the file handle when rioxarray is the one that created it. If you're passed an open file handle then typically the user should be the one to close it.

This only changes the URIManager used in rioxarray.open_rasterio. If I understand what you are saying, I think this should be safe. Additionally, this PR makes the close method functional as previously it just has pass in there.

@snowman2 snowman2 changed the title BUG: Close file handle & pass kwargs with lock=False BUG: Close file handle with lock=False May 24, 2021
@snowman2 snowman2 force-pushed the closefh branch 3 times, most recently from 236f049 to 307614c Compare May 24, 2021 17:25
@snowman2 snowman2 merged commit 6310cca into corteva:master May 24, 2021
@snowman2 snowman2 deleted the closefh branch May 24, 2021 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants