ENH: Add image_like function for SpatialImages

[effigies]

I often find myself just wanting to apply a function to data in an MGHImage or Nifti1Image without discarding the headers and just using x.get_data().

I've implemented this map as a separate function, but it could easily be a method in SpatialImage, allowing subclasses to be more particular about their mapping strategy. Basic test shows that mapping (+1) to an image containing zeros results in all ones.

Is this a thing people would find useful? I'm sure there's more work to be done to integrate it properly into the nibabel framework, but figured I'd start with a proof-of-concept.

[coveralls]

Coverage increased (+0.0%) to 94.33% when pulling 5cc9966 on effigies:data_functor into 96d474c on nipy:master.

[matthew-brett]

Thanks very much for this.

A previous request that I didn't follow up on was to have a function or method called 'image_like' which accepts an image as a template followed by data, optionally affine, header, extra. That would be rather general, so your code could be:

new_img = nib.image_like(img, func(img.get_data())

What do you think?

[effigies]

That could work. Personally, I use the map inline a lot, so I'll most likely still have something like this in my own code:

imageMap = lambda f, x: nib.image_like(x, f(x.get_data())

But whatever seems most fitting for how nibabel does things is fine with me.

I also have a somewhat common pattern of performing folds (or reduces) on the data of several SpatialImages. concat_images seems to do a lot of the plumbing you'd want, which makes sense as concatenation is basically a fold on cons. Would there be interest in something more generic?

Right now, for instance, I use the following:

def sumMGHs(inputs):
    images = [nib.load(i) for i in inputs]
    mgh = np.hstack(i.get_data() for i in images)
    return nib.MGHImage(np.sum(mgh, axis=1).reshape(mgh.shape[0], 1, 1),
                        images[0].get_affine(), images[0].get_header())

What would be ideal would be something like:

sum_images = reduce_images(np.sum, images)

And similarly, the existing function might be rewritten:

concat_images = lambda images, check_affines=True, axis=None: reduce_images(np.concatenate, images, check_affines, axis)

[effigies]

Oh, should note that I'm considering the version of concat_images that appears in #298.

[effigies]

Went ahead and created nib.image_like, since it would certainly be useful. Since the map function didn't generate much enthusiasm, I just replaced it.

Just want to point out that this same function could have (at least) three interfaces:

nib.image_like(img, data)
img.new_image(data)
img.from_image(img, data)

I implemented the first. The code for the second would be identical, except for an indent and changing img to self. The last would simply add a data=None parameter to the from_image classmethod to replace the dataobj.

I think I prefer the first or second, so that its use wouldn't depend on the image having been assigned to a variable or its class being known in advance, but since the functionality is so close to an existing function, it seemed worth noting.

[coveralls]

Coverage increased (+0.94%) to 95.27% when pulling ea578f8 on effigies:data_functor into 96d474c on nipy:master.

[matthew-brett]

In general this does seem a good idea.

One issue is that nibabel does not insist for every supported format that:

• You can easily instantiate an image in memory - e.g. Minc
• If you can easily instantiate an image in memory, we don't insist on the ImageClass(data, affine, header) signature.

For example your image_like on a Minc image or PARRECImage, will fail as things are now.

Thinking aloud, how about:

• an image_like function as you have here, but;
• deferring to a from_image method on the class, that
• has the input img as the first argument, and data as the second optional argument.

?

[effigies]

I like this, as it gives a simple interface while taking advantage of existing behavior. I'll push what I think you're saying.

I'm not sure how to implement this in the subclasses you mention, but I could start by overriding from_image on these classes to do one of the following on if data is not None:

• throw a warning that data parameter will be ignored, return klass.from_image(img)
• throw an exception, since the expected behavior will be violated by just ignoring data

[coveralls]

Coverage increased (+0.94%) to 95.27% when pulling 49cbb2f on effigies:data_functor into 96d474c on nipy:master.

[matthew-brett]

Sorry to be slow to comment on this one.

My slowness, as usual, is due to mental fog when thinking through the API problems.

Now I do think about it, I'm thinking that we need a new SpatialImage method from_dah or similar, with signature:

class SpatialImage:
    ...
    @classmethod
    def from_dah(klass, data, affine, header):
        return klass(data, affine, header)

In this case, images that can't do this (like MINC or PAR / REC) can just refuse:

    @classmethod
    def from_dah(klass, data, affine, header):
        raise NotImplemented('We cannot yet make this type of image from data, affine, header')

What do you think?

[effigies]

Sorry to be slow to respond. I'm not so much opposed to this idea as not quite understanding the reasoning. Could you elaborate a little? (Apologies for asking you to elaborate your thought process >2 weeks later.) Is this "Let's not mess with from_image()" or "We should really have a from_dah()"?

What should from_dah do with img.extra? Or is that something that should not be assumed to be preserved in a mapping?

Finally, it looks like the klass.rw and klass.makeable flags (in #329) seem informative enough to be used to do something like:

@classmethod
def from_dah(klass, data, affine, header):
    if not klass.makeable:
        raise NotImplemented('...')
    elif not klass.rw:
        raise NotImplemented('We cannot yet make this type of image from data, affine, header')
    else:
        return klass(data, affine, header)

Or possibly a separate flag could be defined, rather than overriding the method.

Anyway, my goal here is the image_like function, so whatever class methods it piggybacks on is fine with me. It would just be nice to really understand the underlying issues.

[bcipolli]

@effigies is this something you'd like to push forward on? If so, I've read through the comments and code and I'd be happy to iterate on it with you.

[effigies]

Sure, but it needs some thinking through. My goal when I started this was to put in an image_map function, where I could just apply a function to all points on a dataset and get back a new image with the same affine and the like. That seemed to be asking a bit much of nibabel, and so we scaled back to an image_like where I could write image_map(f, x) = image_like(x, f(x.get_data())).

How to implement that while working with the existing structures is the question. Iterating through a few PRs with you and Matthew has given me a better idea of how things work, but I haven't taken the time to re-think this PR in that context.

Swapping new dataobjs in SpatialImage is a fairly straightforward idea (though apparently MINC and PAR/REC don't work with this, and I still haven't looked at them closely enough to understand why not). Should this stick with that, or should we be more ambitious and try to figure out a structure that would work with any FileBasedImage, and it's up to the image class to figure out how to replace the data?

Possibly this would make sense to incorporate into the data-access (e.g. volumetric/surface) interfaces we were discussing in #360? Because the decisions about how a class should expose data should be very closely related to the ones needed to swap in a new array entirely.

[bcipolli]

Swapping new dataobjs in SpatialImage is a fairly straightforward idea (though apparently MINC and PAR/REC don't work with this, and I still haven't looked at them closely enough to understand why not).

What I understood from above is: if each image declares the x/y/z/t axes, then this is straightforward (even for MINC and PAR/REC). I don't think it's wise to assume axes; it may be implementation-dependent on each image class. Heck, some classes under consideration may contain more than one image! So, better to declare those explicitly, I think.

And I think, from an informatics point of view, declaring axes explicitly is a big win; if those axes are implicit (and from the discussion above I believe they are), then informatics code has to actually check the image type.

Edit:
Perhaps axes definitions should be a property, as some formats may contain that info in headers; for others it would simply be hard-coded?

Should this stick with that, or should we be more ambitious and try to figure out a structure that would work with any FileBasedImage, and it's up to the image class to figure out how to replace the data?

FileBasedImage does not refer to storage at all; it's just an abstract interface to define file access. I think it would be a mistake to define the property there.

While there could be another interface (or mixin) for something like this, I suggest aiming at SpatialImage first. It's more constrained and clear. Even if we wanted to go across all image types, I'd suggest doing a PR strictly on SpatialImage first, to keep things manageable.

Iterating through a few PRs with you and Matthew has given me a better idea of how things work

Same here. I still feel very tentative in the things I suggest, but this is my best idea on my current understanding.

[effigies]

I've rebased this.

To clarify my comments from #372, you actually wouldn't use image_like to convert between classes at all. If you wanted to do something like what I said, then you'd instead do Nifti1Image.from_image(mincimage, dataobj), while image_like is a standalone that uses the input image to determine the output image type.

I'm not quite following the axis discussion. Are you thinking that this should produce an image in the image format's "preferred" axis order? While that seems like a valid option for from_image, (possibly normalize_axes?), I think image_like should be as unchanging as possible, as reshaping incoming data would be exactly the opposite of my expectation.

Let me know what you think.

[bcipolli]

I am honestly a little baffled over my previous comments; I can't find any references to x/y/z/t axes elsewhere, but I swear I was responding to previous comments about it. My memory traces fade faster than I'd hope :)

I'm taking some time to read through the docs a bit more deeply, given my misunderstanding at #372, so that I can think through this (very light-weight) code and why it may not work in some instances. Hope to comment soon.

[matthew-brett]

To be specific about why this does not work in general - consider this:

In [15]: from os.path import dirname, join as pjoin

In [16]: import nibabel as nib

In [17]: minc_fname = pjoin(dirname(nib.__file__), 'tests', 'data', 'minc1_4d.mnc')

In [18]: minc_img = nib.load(minc_fname)

In [20]: minc_data = minc_img.get_data()

In [21]: minc_data.shape
Out[21]: (2, 10, 20, 20)

The first dimension in this data array is time. Now if I do:

In [26]: nifti_fname = pjoin(dirname(nib.__file__), 'tests', 'data', 'example4d.nii.gz')

In [27]: nifti_img = nib.load(nifti_fname)

I can run image_like(nifti_img, minc_data), without complaint, but the result will have shape (2, 10, 20, 20). I might assume, because this is a Nifti image, that I can slice over the last dimension to get time volumes (this is what the nifti standard says), or I might save this image to disk and some other software, Python or otherwise, won't realize that the time axis is first and make a mess.

So we need to either let people shoot themselves in the foot in this way, or maybe do something like keep the axis information with the data, so that the image_like function can know to transpose in this situation...

[effigies]

Wrapping axis information could start getting messy, but here's a halfway measure: Check that the dimensions of the data object match those of the starting image in the spatial directions. The class knows about its own spatial axes, but there's no need to infect raw numpy arrays with this information.

There's a possibility of a situation where minc_img.shape[:3] == nii_img.shape[:3], but that seems like acceptably low probability to me.

The primary pattern is retaining metadata for within-format analysis pipelines:

img = nib.load(fname)
res = func(img.get_data())
img.__class__(res, img.affine, img.header.copy()).to_filename(new_fname)

I'm perfectly happy to add documentation to make clear that using it as a conversion routine is not recommended.

[nibotmi]

☔ The latest upstream changes (presumably #404) made this pull request unmergeable. Please resolve the merge conflicts.

[coveralls]

Coverage increased (+0.002%) to 96.255% when pulling 0306b6b on effigies:data_functor into b7eee37 on nipy:master.

[codecov-io]

Codecov Report

Merging #300 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #300      +/-   ##
==========================================
+ Coverage   88.38%   88.38%   +<.01%     
==========================================
  Files         188      188              
  Lines       24008    24016       +8     
  Branches     4254     4254              
==========================================
+ Hits        21219    21227       +8     
  Misses       2102     2102              
  Partials      687      687

Impacted Files	Coverage Δ
nibabel/spatialimages.py	96.39% <100%> (+0.05%)	⬆️
nibabel/__init__.py	93.75% <100%> (+0.13%)	⬆️
venv/Lib/site-packages/nibabel/spatialimages.py	96.39% <0%> (+0.05%)	⬆️
venv/Lib/site-packages/nibabel/__init__.py	91.66% <0%> (+0.17%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97447e2...cc5b2bb. Read the comment docs.

[coveralls]

Coverage increased (+0.002%) to 96.256% when pulling facd122 on effigies:data_functor into d420dcf on nipy:master.

[coveralls]

Coverage decreased (-1.2%) to 91.834% when pulling 80012ec on effigies:data_functor into 41e126a on nipy:master.

[nibotmi]

☔ The latest upstream changes (presumably #550) made this pull request unmergeable. Please resolve the merge conflicts.