Skip to content

Add compression to LocalFileOpener #660

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

Merged
merged 3 commits into from
Jun 8, 2021

Conversation

limx0
Copy link
Contributor

@limx0 limx0 commented Jun 7, 2021

This PR copies the compression implementation from OpenFile to LocalFileOpener to allow the compression kwarg to work for local files again.

I've added a simple test also. This should fix #659

@martindurant
Copy link
Member

OK, I think this works fine. Note that fsspec.open does already handle this, and also explicitly keeps references to the intermediate file objects so that they can be cleanly closed. I am supposing that garbage collection is doing the right thing here, as the compressed file keeps a reference to the raw file.

@martindurant martindurant merged commit 0f700d4 into fsspec:master Jun 8, 2021
@limx0
Copy link
Contributor Author

limx0 commented Jun 8, 2021

Thanks @martindurant - I'm doing some other operations with an open filesystem so this is just how I ended up opening the file (as opposed to just using fsspec.open). Should I add some extra testing to ensure these files are cleaned up?

@martindurant
Copy link
Member

I think it's OK. The extra tests would need some mocking or other inner hacks that might be unpleasant.

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.

compression="infer" broken for localfilesystem?
2 participants