Skip to content

nrrd support #356

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

Open
satra opened this issue Oct 9, 2015 · 17 comments
Open

nrrd support #356

satra opened this issue Oct 9, 2015 · 17 comments

Comments

@satra
Copy link
Member

satra commented Oct 9, 2015

@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
Copy link

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
Copy link
Member

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
Copy link

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
Copy link
Member

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
Copy link

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
Copy link
Member

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
Copy link

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
Copy link
Member

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

@TheChymera
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

@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
Copy link
Member

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
Copy link

lassoan commented Nov 30, 2022

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
Copy link
Member

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 (#356 (comment)) 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:

  • FileBasedImage: Abstract class that defines IO interface (e.g., to_filename, from_filename), links to a header
  • DataobjImage: Provides (data, header) model. Data objects and methods handle caching, memory mapping and on-demand I/O
  • SpatialImage: Provides consistent (data, affine, header) interface

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
Copy link

lassoan commented Nov 30, 2022

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
Copy link
Member

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:

nibabel/setup.cfg

Lines 15 to 18 in d33a05a

minc2 =
h5py
spm =
scipy

@effigies
Copy link
Member

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

nibabel/pyproject.toml

Lines 60 to 61 in 7327927

minc2 = ["h5py"]
spm = ["scipy"]

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

No branches or pull requests

8 participants