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

Gzip #1

Merged
merged 4 commits into from Mar 4, 2014
Merged

Gzip #1

merged 4 commits into from Mar 4, 2014

Conversation

rafamanzo
Copy link
Contributor

Hi there!

First of all thanks for the gem!

I've changed somethings for comability with ruby 2.1.0 and read and write support for gzip files.

rcov replaced by simplecov
spec/nifti/n_write_spec.rb:35 precedence of arguments fixed
If the extension of the given file to save is .gz, saves it according to it
@kastman
Copy link
Collaborator

kastman commented Mar 3, 2014

Hey Rafael,

Thanks for this! The Nifti gem has definitely been neglected and I appreciate the fixes; gzip support is especially helpful. A lot has changed since I last worked on this (the Yehuda post about Gemfile.lock had only been published for 5 months at the time of the last commit on this ;)

My only question with this PR is the checkin of the .ruby-version at 2.1.0. Does committing a .ruby-version essentially mean that the gem cannot be used in other versions (since at least bundler, and the common environment managers rbenv/rvm respect it), or is there some way to allow both 1.9 and 2.0? Is it ignored except during development, etc? Sorry, but I've been a bit out of touch lately.

I'm having trouble install 2.1.0 with rbenv, so I can't check your tests at the moment, but everything looks great. Thanks for fixing the syntax error in mine as well. :)

Best,
Erik

@rafamanzo
Copy link
Contributor Author

The scientific ruby as a whole has been neglected as whole on the last couple of years :/

But I hope that with NMatrix being part of gsoc 2014, things might change. Specially because having to use python for scientific compute, when writing automated tests is a real pain compared to ruby in my opinion :)

The .ruby-version (and .ruby-gemset) only affects your development environment and only if you are using a ruby manager like RVM (http://rvm.io). I have my Ruby 2.1.0 installed through this manager :)

Thank you, Rafael.

kastman added a commit that referenced this pull request Mar 4, 2014
@kastman kastman merged commit c877fec into brainmap:master Mar 4, 2014
@kastman
Copy link
Collaborator

kastman commented Mar 4, 2014

Totally agree - anything we can do to increase usage in the scientific community is always appreciated!

I saw that you're adapting a branch with NMatrix - please let me know if I can help with that at all. I included an option for NArray, but it seems like that's been supplanted?

I'm accepting the PR with the .ruby-version file included but will probably remove it in a separate commit. It's fine for development, but (like Gemfile.lock) I don't think it should really be committed in the repo. I'm still having trouble getting rbenv to install 2.1.0, but I think that's a local problem. :)

Thanks again for your help! Let me know if there are any other thoughts you have to keep this gem up to date.

-Erik

@rafamanzo rafamanzo deleted the gzip branch March 4, 2014 11:48
@rafamanzo
Copy link
Contributor Author

Hi Erik,

Yes, NMatrix is the successor of NArray as far as I know. But on that experience that I made replacing NArray by NMatrix, I confirmed that NMatrix still on an early alpha. First it does not compile with Ruby 2.x (SciRuby/nmatrix#178) which has been fixed but not yet released. Then, with ruby downgraded to 1.9.3, I had to install ATLAS instead of blas and lapack, which was difficult especially because I had to change the settings for my CPU power saving.

With NMatrix finally installed and with the code modified like you can see on that branch I ran the tests to receive a beautiful segfault after all that! Hahaha
I've just kept the branch there in the hope that with the RC2 of NMAtrix those things get fixed and it would work. But, for now I think NArray still a better solution.

About the .ruby-version, agreed :)

About my thoughts for the gem, I've created a new class, named NImage, which makes easier to handle the image data. Specifically, instead of viewing the data as a large one-dimensional array, it makes possible to deal with the image as an n-dimensional according to it's shape on the dim field of the header.
On my manual tests it worked well and I hope to soon submit new PR with that new class. I just have to cover it with unit tests, and write the documentation like you did for the other classes.

One more thing that has been concerning me since the beginning is about the performance for retrieving the image data. Is there anything that we can do to improve it?
Retrieving a dataset 140x140x60x6 takes something like 40s on my Core 2 Duo, while with Python's NiBabel it takes less than 1s.

Thank you for your receptiveness!

Rafael.

@kastman
Copy link
Collaborator

kastman commented Mar 5, 2014

Ugh - it sounds like you still have the patience I lost years ago! It's a shame that NMatrix is still lacking, but I share your enthusiasm that scientific ruby may be picking up momentum and reaching critical mass. Changing your power settings to compile math libraries is tragic!

I have to ask some questions to the sciruby/bioruby folks for some other reasons, and I'll see if I can't bring this up as well.

I'm adding a new issue for the load time of image data - I remember thinking it was ridiculously slow and it is definitely worth trying to find the bottleneck. I'll see if I can't find some time to track that down.

@rafamanzo
Copy link
Contributor Author

Yes, my patience now can be credited to my lack of patience with python after six months developing the code for my MSc dissertation there. Hahaha

It has really great scientific libraries (the best I think...), but it lacks a good test framework like RSpec and I hate those init.py polluting my source tree. So I really want to port it to ruby and in order to do that I needed a way to read/write nifti files, so I searched in rubygems and here I'm!

If you could ask the sciruby/bioruby community about any thoughts on this would be great as well as if you could point out the bottleneck on reading image data. Thanks a lot! I'm not experienced with the nifti format, but perhaps with you pointing out the bottleneck, I could help with my ruby experience :)

Thanks again!

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.

None yet

2 participants