NiftiWidget can take in nibabel image

[cancan101]

NiftiWidget should also allowed to be constructing providing the nibabel image. Right now only a path is allowed. load is this called on the path. I suggest allowing the user to pass in the same (ie the result of the user having called load himself) into NiftiWidget.

Alternatively, NiftiWidget can be made to subclass VolumetricWidget (handles more than nii files) and then VolumetricWidget takes in the nibabel image.

[janfreyberg]

Yep, very good point. Am going to add this ASAP. Unless you want to make a PR - it's currently being handled well by the SurfaceWidget, so just taking that across would be easy!

I think I'd prefer to keep a single interface that checks what the input is and handles it accordingly.

[cancan101]

I was going to PR this, but if you want go ahead. That being said, I assume the change for volume will happen in constructor rather than after the fact?

[cancan101]

You might also need this change too:

@@ -103,8 +103,10 @@ class NiftiWidget:
         plt.gcf().clear()
         plt.ioff()  # disable interactive mode

-        data_array = self.data.dataobj.get_unscaled()
-
+        if hasattr(self.data.dataobj, 'get_unscaled'):
+            data_array = self.data.dataobj.get_unscaled()
+        else:
+            data_array = self.data.dataobj
         if not ((data_array.ndim == 3) or (data_array.ndim == 4)):
             raise ValueError('Input image should be 3D or 4D')

[janfreyberg]

Hey, just getting around to doing some catch up with issues.

I think you're right, the change would be in NiftiWidget.__init__().

A simple check as to wether it's a path / string or a nib.NiftiImage instance would sort it out. Then just set self.filename to None and self.data to the nibabel image.

I'm not sure I fully understand the other change you're proposing - in which cases do images not have a get_unscaled() method? (NB: This might also be due to my relative ignorance of nibabel).

[cancan101]

For example if you use resample_img from nilearn.image, the dataobj just becomes a numpy array itself so you can't access get_unscaled.

Alternatively you can just use get_data() on the image itself to get an array.

[janfreyberg]

Ah OK, that's interesting.

Yeah, maybe get_data() would be the better option - more foolproof. Can't exactly remember why I used the get_unscaled() method in the first place.

Do you want to take these changes on?

[cancan101]

Here are the notes in docs:

If you want unscaled data, please use img.dataobj.get_unscaled() instead. If you want scaled data, use img.get_data() (which will cache the loaded array) or np.array(img.dataobj) (which won’t cache the array).

and:

Less commonly, for some image types that support it, you might want to fetch out the unscaled array via the object containing the data:

unscaled_data = img.dataoobj.get_unscaled()
Analyze-type images (including nifti) support this, but others may not (MINC, for example).

[janfreyberg]

Doesn't seem obvious to me that the widgets should use the unscaled data.

[cancan101]

@janfreyberg any reason to save filename: self.filename = Path(filename).resolve() as opposed to making it a local variable?

[janfreyberg]

It's a remnant from when the data was loaded not on initialisation but later on - so not anymore. Although I think it could be nice to add a __repr__ method in the future that displays some sort of "source" information - so maybe this should be changed to self.source, and when using a niftifile, it displays something like "preloaded".

But we can think about that when we actually implement a __repr__ method.

[cancan101]

I put up two PRs the first depends on the second.

[janfreyberg]

Brilliant, thanks!