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

Allow Shapefile to read .zip files #113

Merged
merged 8 commits into from
Apr 27, 2024
Merged

Allow Shapefile to read .zip files #113

merged 8 commits into from
Apr 27, 2024

Conversation

asinghvi17
Copy link
Member

@asinghvi17 asinghvi17 commented Apr 26, 2024

This is an implementation of #75 that "dispatches" on provided file name. Ideally, this should also work with some form of streaming type so that we can pass that to ZipFile and thus use cloud shapefiles directly, but this will do for now.

The PR implements an extension ShapefileZipFilesExt which provides a method for a function _read_shp_from_zipfile that is defined as a function with no methods in Shapefile.jl. This also allows us to provide a nice error message if the extension is not loaded.

CC: @dgleich

We should subscribe to the FileIO interface so we can deal with streaming files as well...

Co-authored-by: David Gleich <[email protected]>
@asinghvi17 asinghvi17 requested a review from rafaqz April 26, 2024 00:52
@asinghvi17
Copy link
Member Author

Test failures seem a bit strange...

@dgleich
Copy link
Contributor

dgleich commented Apr 26, 2024

Oh, this is awesome! Thanks for pushing it through!

@rafaqz
Copy link
Member

rafaqz commented Apr 26, 2024

@asinghvi17
Copy link
Member Author

Ah! Should I fix the tests then?

@rafaqz
Copy link
Member

rafaqz commented Apr 26, 2024

Yes! lets look out for little breaking changes like that next time and bump the minor version ;)

@asinghvi17
Copy link
Member Author

asinghvi17 commented Apr 26, 2024

CI passes now. Should we merge and release? I've updated the minor version (so this would technically be a breaking release), even if nothing is actually going to break.

@rafaqz
Copy link
Member

rafaqz commented Apr 26, 2024

What about .gz and every other zip format? .zip is pretty windows centric?

@asinghvi17
Copy link
Member Author

It seems that ZipFile.jl only handles .zip files. We could switch to TranscodingStreams but I have no clue how (or if) it handles files...

@dgleich
Copy link
Contributor

dgleich commented Apr 27, 2024

There are a lot of places that distribute shape files as zip files (e.g. this seems to be how the US government does it), so this provides benefit as is... i.e you can download the zip file and read it directly.

(My original use case involved working with a few hundred such zip files, which I really didn't all want to unzip...)

The TarFiles situation isn't as straightforward as there isn't (yet) a straightforward way to read a tar.gz file as a list of Julia IO objects as there is for zip files. See, e.g. JuliaIO/Tar.jl#95 So I wouldn't worry about .tar.gz files right now.

@rafaqz
Copy link
Member

rafaqz commented Apr 27, 2024

Makes sense. If zip is 95% of the files and the only thing thats easy then thats fine.

@asinghvi17
Copy link
Member Author

Do we need more reviews or should I merge + release?

ext/ShapefileMakieExt.jl Outdated Show resolved Hide resolved
@asinghvi17 asinghvi17 merged commit 35c8d42 into main Apr 27, 2024
14 checks passed
@asinghvi17 asinghvi17 deleted the as/zipfile branch April 27, 2024 12:35
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