nipy / nibabel

Python package to access a cacophony of neuro-imaging file formats
http://nipy.org/nibabel/
Other
647 stars 257 forks source link

nrrd support #356

Open satra opened 8 years ago

satra commented 8 years ago

@matthew-brett - hans (@hjmjohnson) sent this email, and i think the best is to continue the discussion here.


I have worked on the pynrrd project ( a pure python NRRD reader) to make it compatible with dealing with DWI data. It would be great to help analyze data that is processed with NAMIC style tools in DIPY style tools.

https://github.com/mhe/pynrrd

Here is the test script we used to verify that we could read NRRD files, and convert them to equivalent nifti version when read by nibabel. https://github.com/BRAINSia/pynrrd/blob/ReadNAMICStyleDWIData/testNrrd.py

Could you please advise me on the best strategy for integrating this into nibabel?

Thank you, Hans

hjmjohnson commented 8 years ago

Thank you Satra. I appreciate the introductions.

Hans

From: Satrajit Ghosh Reply-To: nipy/nibabel Date: Friday, October 9, 2015 at 6:33 PM To: nipy/nibabel Cc: Hans Johnson Subject: [nibabel] nrrd support (#356)

@matthew-brett - hans (@hjmjohnson) sent this email, and i think the best is to continue the discussion here.

I have worked on the pynrrd project ( a pure python NRRD reader) to make it compatible with dealing with DWI data. It would be great to help analyze data that is processed with NAMIC style tools in DIPY style tools.

https://github.com/mhe/pynrrd

Here is the test script we used to verify that we could read NRRD files, and convert them to equivalent nifti version when read by nibabel. https://github.com/BRAINSia/pynrrd/blob/ReadNAMICStyleDWIData/testNrrd.py

Could you please advise me on the best strategy for integrating this into nibabel?

Thank you, Hans

— Reply to this email directly or view it on GitHub.

matthew-brett commented 8 years ago

Sorry to be so slow to reply, I've been swamped this last couple of weeks teaching and releasing another package.

I'm sorry about this, but the general strategy is to build up a Image and Header class for Nrrd images.

I'm happy to help, I will probably have to help because it's not at all obvious at the moment.

The basic idea is that you subclass the nibabel.spatialimages.Header object or make a class with the same API, to contain the Nrrd header. Then subclass `nibabel.spatialimages.SpatialImage to make the Nrrd image.

To see whether you've got that working right, add tests for your new Nrrd class in nibabel/tests/test_image_api.py - there are examples in there that my help.

It's great you have some example images in your tests directory, that will help a lot.

It can also be useful to add larger test files into their own test data repository - see : http://nipy.org/nibabel/devel/add_test_data.html

Thanks a lot for the suggestion - I am sure Nrrd support will be very useful.

hjmjohnson commented 8 years ago

Thank you Matthew. I have a grant due over the next week, so I won’t be able to work on this issue until after October 9th.

I appreciate your comments, and I sincerely hope that we can integrate pynrrd with nibabel for supporting diffusion imaging.

Regards, Hans

From: Matthew Brett notifications@github.com Reply-To: nipy/nibabel reply@reply.github.com Date: Tuesday, October 20, 2015 at 2:12 PM To: nipy/nibabel nibabel@noreply.github.com Cc: Hans Johnson hans.j.johnson@gmail.com Subject: Re: [nibabel] nrrd support (#356)

Sorry to be so slow to reply, I've been swamped this last couple of weeks teaching and releasing another package.

I'm sorry about this, but the general strategy is to build up a Image and Header class for Nrrd images.

I'm happy to help, I will probably have to help because it's not at all obvious at the moment.

The basic idea is that you subclass the nibabel.spatialimages.Header object or make a class with the same API, to contain the Nrrd header. Then subclass `nibabel.spatialimages.SpatialImage to make the Nrrd image.

To see whether you've got that working right, add tests for your new Nrrd class in nibabel/tests/test_image_api.py - there are examples in there that my help.

It's great you have some example images in your tests directory, that will help a lot.

It can also be useful to add larger test files into their own test data repository - see : http://nipy.org/nibabel/devel/add_test_data.html

Thanks a lot for the suggestion - I am sure Nrrd support will be very useful.

— Reply to this email directly or view it on GitHub.

matthew-brett commented 8 years ago

On Wed, Oct 21, 2015 at 7:32 AM, Hans Johnson notifications@github.com wrote:

Thank you Matthew. I have a grant due over the next week, so I won’t be able to work on this issue until after October 9th.

I appreciate your comments, and I sincerely hope that we can integrate pynrrd with nibabel for supporting diffusion imaging.

I guess you mean November 9th - no problem. That suits me fine too because I'm in Cuba until the 2nd. When you get going, please do start a WIP (work in progress) PR for your work, when you'd like comments as you go.

Thanks again.

ihnorton commented 8 years ago

I've checked-in with @hjmjohnson, and plan to work on this.

I will probably have implementation questions later, but a preliminary question first: I see nibabel (conditionally?) depends on pydicom -- what is the policy / preference on external dependencies?

Pynrrd is in PyPi, but the tagged version does not include Hans's changes, and the author hasn't responded to an update request from August (bumped by me a week ago). My inclination is to pull in the code (~500 lines, MIT licensed) to avoid the dependency, and allow more flexibility in re-working as needed to fit nibabel.

matthew-brett commented 8 years ago

That sounds reasonable. I'm sure you'll have to refactor that code a lot anyway because of the particular structure of the Nibabel image model.

Please ask liberally on issues and Work In Progress pull requests, I've got some time over the holidays and I will do my best to give feedback in good time.

matteawelch commented 7 years ago

Hi, Has any work has been done on this issue to accommodate nrrd compatibility? I recently tried to use the nipype distance measure for two contours saved in an nrrd format and got the following error: ImageFileError: Cannot work out file type of "./Contour1_IndexedLabelmap.nrrd" Interface Distance failed to run. Thanks for any information you can give! Mattea

effigies commented 7 years ago

Hi Mattea. Nibabel doesn't currently support NRRD. We'd need a contribution from someone familiar with the format.

TheChymera commented 6 years ago

I'd also be very happy to use this feature - for histological tracer and gene expression maps from the Allen Brain Institute, which come delivered as .nrrd files.

effigies commented 6 years ago

So it looks like @hjmjohnson's updates to pynrrd were incorporated, which I assume is why this stalled out. Have you looked into that?

Would still welcome a contribution, but it's going to take some initiative from someone who's familiar with the format, or at least the pynrrd and nibabel image APIs. @TheChymera are you up to give it a shot?

TheChymera commented 6 years ago

@effigies I literally don't know anything about this format, and I have to stop spreading myself so thin... ^^ but I will be looking into pynrrd. If it's easy and you don't mind adding pynrrd to nibabel's deps, I might actually try and write up something.

effigies commented 6 years ago

Yeah. Absolutely get your stuff done first. If you find some time to open a PR, cool. If not, we've survived without it for 2+ years.

lassoan commented 1 year ago

We would like to use NRRD instead in our pipelines, as it seems to work much more robustly than NIFTI (with all the SFORM/QFORM inconsistency issues). Many packages uses nibabel for image reading/writing, so it would be a convenient solution if nibabel supported NRRD directly.

What is the current state of NRRD support in nibabel?

We use the NRRD file format a lot, so we can help with any questions, testing, or even implementation.

effigies commented 1 year ago

Hi @lassoan, in general implementation will go faster if led by a motivated contributor who has access to the format and a need to work with it. Matthew's comment here (https://github.com/nipy/nibabel/issues/356#issuecomment-149672387) still applies as to how we would probably suggest going about it.

If you're interested, the classes you'll want to inherit from will be:

Inheriting from SpatialImage will get you the other two for free. You'll probably want to look into the ArrayProxy class for handling your data objects.

I don't know anything about nrrd to provide more context, but I'm happy to help guide. If you start writing code, feel free to open a PR as a work-in-progress early to ask concrete questions.

lassoan commented 1 year ago

Thank you, I'll put this on my TODO list.

Is it OK to add pynrrd (with its trivial requirements) as a dependency to nibabel?

effigies commented 1 year ago

It is light, but my experience is that it doesn't take much for your dependency graph to start getting out of hand. For users who are not using NRRD files (everybody so far...), this will add one more thing to download. Being a core library, we should be very hesitant to add mandatory dependencies.

We currently have extras that install additional components for different formats, so nibabel[dicom,spm] will additionally install pydicom and scipy, and nibabel[minc2] will install h5py. I would suggest you add nrrd to install pynrrd here:

https://github.com/nipy/nibabel/blob/d33a05a1e097985226f4c3a6d15d163cd0204af0/setup.cfg#L15-L18

effigies commented 1 year ago

For anybody looking to work on this, the optional dependencies are now in pyproject.toml:

https://github.com/nipy/nibabel/blob/7327927fb9590dcb854e41203412da91d372d62d/pyproject.toml#L60-L61