[Pkg-exppsy-pynifti] [pynifti] NiftiDatatset bug?
michael.hanke at gmail.com
Tue Jan 13 19:18:24 UTC 2009
On Tue, Jan 13, 2009 at 01:14:38PM -0500, Scott wrote:
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.
> 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
> * 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.
> * 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
> * on save, an option to force removal of singleton dimensions and/or
> explicitly set dim. I believe some of the freesurfer volume tools
> actually throw an error if dim = 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.
> * (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.
GPG key: 1024D/3144BE0F Michael Hanke
More information about the Pkg-exppsy-pynifti