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

libzip based zip file handling #41

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

leezu
Copy link

@leezu leezu commented Oct 8, 2018

This removes the custom zip file handling code and uses libzip instead. This addresses the missing Zip64 support (ie. the inability to write npz files with large arrays; #39). For ease of usage, the libzip++ C++ bindings are used. A submodule with the libzip++ header is included and cmake is set up to make sure the submodule is automatically initialized. This uses some C++14 features, but support should be sufficiently widespread by now I guess.
I believe it is simpler and more safe to rely on libzip instead of adding custom code to cnpy to handle Zip64 support while remaining compatible with the currently used Zip format. libzip handles this and other Zip extensions transparently.

This should also fix reading the header_length field in parse_npy_header on big-endian systems.

If you're unhappy with introducing new dependencies and raising the requirement, feel free to close this. I will keep the code around as a fork for anyone that is willing to make the trade-off and requires Zip64 support. But of course it would be great if this can be merged in some way so that other people don't run into similar issues. Please let me know what you think. I may add some more documentation in case you would like to merge this at some point.

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.

1 participant