[Pkg-exppsy-pynifti] [pynifti] NiftiDatatset bug?

[Michael Hanke]

Hi Scott,

On Tue, Jan 13, 2009 at 01:14:38PM -0500, Scott wrote:

<snip>
My reply to all that is: got it! ;-)

>> Is this somewhat helpful or even touches to core of your problem?
>>
>>   
> yes, now we're talking! but let me stress that all the examples i've  
> given *are* use-cases that i've come across in my own programs, where  
> niftiimage doesn't handle ambiguity in the array shape gracefully. Sure,  
> at any given line, two more lines of numpy code will solve the problem -  
> but this forces every function to solve the problem! Every time I load  
> niftidata, I must ensure it's the shape I expect for that function and  
> usage. So two lines of code becomes ... well ... proportional to your  
> use of niftiimage. The bigger issue is that everyone will occasionally  
> forget to write those two lines of code, and newbies won't know they  
> need to. To me this screams for wrapping those two lines into the class  
> def itself.
Ack.

> i do agree that NiftiImage should be treated as a data object and expose  
> just what's necessary for working with the data. But for me, necessary  
> tools include those for concatenating/indexing data in a consistent  
> manner, especially since nifti is designed for a large variety of  
> dimensionality, including multiple samples (not to mention mapping  
> between dimensions, like 3d ROI's across time). So, for example, let me  
> explicitly (re)propose the following set of tools which, IMHO, would  
> enable me to treat a NiftiImage as a proper data object without making  
> assumptions about its shape:
>
> * concatenation which automagically upcasts to the minimum  
> dimensionality required, and no more. So, eg, I could call nim =  
> nim.extend(NiftiImage(f) for f in file_list, dim='t') and never have to  
> worry about what the data arrays look like
Ack. Although I rather see that as a utility function and not a class
method.

> * indexing which both upcasts or downcasts to the dimensionality  
> required, perhaps dependent on a kwarg, and guarantees you index into  
> the dimension you intend. For instance, nim.data[samples] only works  
> correctly if nim is 4d (otherwise it samples Z), and it will return 3d  
> if samples is a number and 4d if samples is an array. We've already seen  
> this bug in pymvpa... Ideally there should be something like  
> nim.get_timeseries(times=':')->[t x Z x Y x X] or one of the other  
> syntaxes i've proposed.
Ack.

> * perhaps a volume iterator across all non-space dims, or a single given  
> non-space dim (default time) which always yields a 3d vol regardless of  
> the original dimensionality. like, for vol in nim.timepoints():
Ack, but how should the volume iterator handle images with more than 4
dimensions?

> * on save, an option to force removal of singleton dimensions and/or  
> explicitly set dim[0]. I believe some of the freesurfer volume tools  
> actually throw an error if dim[0] = 4 even if there's only one  
> timepoint, and this may be true with other packages. again, this can be  
> done manually, but it would be simpler in the class def. This also will  
> add a note about this type of compatibility issue into the docstring for  
> the save method, which could be *really* helpful. I seem to recall this  
> particular nasty requiring an hour of debugging once... Alternatively,  
> just a general method to reshape the niftiimage would suffice.
Ack.

> * (unrelated but fun) perhaps overload basic arithmetic operations that  
> just expose the data array. for instance, often i need an example  
> functional image from a timeseries. there are many ways to do this, but  
> a really nice one would be NiftiImage(myfile).mean().save('exf.nii').  
> Also a nice way to mask an image would be (NiftiImage(myfile) *  
> NiftiImage(mymask)).save(output). These are just conveniences though...
Ack, but that might also be better written as a utility function.

> plus, if these types of things are implemented in the class, the header  
> can be updated as necessary when the data shape changes. i believe (not  
> double checking ;)) this is currently possible only if the data array is  
> extracted, manipulated, then passed into a new niftiimage. so, again,  
> not a big deal, but doing it this way feels more elegant to me, and is a  
> bit more convenient and object-oriented.
The problem with in-place modification of the data is that the Python
object is merely a slim wrapper around the C data structure. Just
checking pre and post conditions during creation of images and export of
data is _way_ easier than keeping all that structural integrity during
in-place modifications. I fear that the added level of convenience is
eaten up but the potential for bugs.


However, I will consider all of your suggestions/proposal as TODO and
will add whatever I can.


> i'll agree/cede with you that there is no need to force the data array  
> to a given shape in general, and 'volumes' can just be stored as (z,y,x).
>
> I guess that at the heart of it, due to how the data array is so  
> malleable, I feel like it should be privately handled to the fullest  
> extent possible... nifti really feels more like a data *container* to me  
It is a container! BTW: with the next release, you'll be able to embed
any serializable Python object into any NIfTI file ;-)

> anyway, given that it can take so many dims. i know nothing i'm  
> proposing will require more than a few lines per method implementation,  
> but I strongly feel that it makes for a more elegant, robust, class  
> design, and yet the full data array will always be exposed for those who  
> wish to use it manually.
>
> oh btw, i'm not sure how someone leading two open source projects (that  
> I know of) gets away with calling himself lazy ;)
Ha! As we are speaking -- since you are full of energy -- do you want
commit access to the repository so you can express your ideas in code
instead of having to convince me with a book of emails?

We usually develop with developer-specific branches, so you'd be free to
do _anything_ and I would be free to simply merge whatever I like ;-)

How does that sound?

> given how vocal we both are about what amounts to a few lines of code,  
> i'm surprised no one else has chimed in... are there other thoughts  
> floating around? anyone else vehemently (dis)agree with me?
I guess that is due to the fact that almost nobody did subscribe to the
new list yet...


Thanks a lot for being patient with me.



Michael


-- 
GPG key:  1024D/3144BE0F Michael Hanke
http://apsy.gse.uni-magdeburg.de/hanke
ICQ: 48230050