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

Fixed open mode from w9 to w #48

Merged
merged 7 commits into from
Jan 7, 2021

Conversation

itan1
Copy link
Contributor

@itan1 itan1 commented Jan 5, 2021

When changing from GZip to CodecZlib the gzopen() method was replaced with open(), but the mode is still "w9". The gzopen() accepts a compression level number in the specified mode, but open() does not. Therefor I changed it to "w".

Fix for Issue #47

@Tokazama
Copy link
Member

Tokazama commented Jan 5, 2021

Thanks! This must have slipped in when switching to the new gz compressor. Would you mind just adding a quick test to make sure we don't accidently break writing compressed files in the future?

@itan1
Copy link
Contributor Author

itan1 commented Jan 6, 2021

Yes, sure! Shall I also remove the files test/example4d.* while I'm on it? I think they are just duplicates of the test/data/example4d.* files which are used in runtests.jl?

@Tokazama
Copy link
Member

Tokazama commented Jan 7, 2021

Shall I also remove the files test/example4d.* while I'm on it? I think they are just duplicates of the test/data/example4d.* files which are used in runtests.jl?

I hope you don't mind, but I went ahead and changed the commit to solve this with a temporary directory that should get cleaned up once julia exits.

This looks good to me. Let me know if you want to make any other changes, otherwise I'll merge.

@itan1
Copy link
Contributor Author

itan1 commented Jan 7, 2021

Ah, yes good idea. Thanks.

Ready to merge from my side 👍

@Tokazama Tokazama merged commit e72842f into JuliaNeuroscience:master Jan 7, 2021
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