Skip to content

added version 2 support for reading - #32

Merged
fhs merged 4 commits into
fhs:masterfrom
MRedies:master
Feb 26, 2019
Merged

added version 2 support for reading#32
fhs merged 4 commits into
fhs:masterfrom
MRedies:master

Conversation

@MRedies

@MRedies MRedies commented Feb 25, 2019

Copy link
Copy Markdown
Contributor

I introduced support for reading version 2 *.npy files. This should fix:

#31

I hope you like it like this.

@fhs fhs left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npzwritearray should check if length(dict) overflows UInt16 or UInt32. If it overflows Uint32, it's an error. Otherwise, if it overflows UInt16, it should use v2 format. Since this PR is focused on reading, maybe we'll do this in a separate PR.

Can you also add a test? Maybe add a small v2 format npz file in the test directory and try to read it. Thanks.

Comment thread src/NPZ.jl Outdated
end
@compat b = read!(f, Vector{UInt8}(undef, length(Version)))
if b != Version
if b[2] != Version[2]

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can safely remove this check. We don't care about minor version number. If version 2.1 ever comes out, it'll probably be backward compatible 2.0.

@MRedies

MRedies commented Feb 26, 2019

Copy link
Copy Markdown
Contributor Author

I added a small test file for v2 and removed the testing of the minor version. Thanks!

@fhs
fhs merged commit dc86987 into fhs:master Feb 26, 2019
@fhs

fhs commented Feb 26, 2019

Copy link
Copy Markdown
Owner

Thanks!

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.

2 participants