ecat image importer

[cindeem]

Hi all..
I finally found the time to refactor/rework some old ecat code to work with latest nibabel master.

It can load data either by frame (the ecat format can hold more than one frame)
Or
will load all the data into a 4D volume (in which case it will check validity of the affine)
It currently only reads Ecat7 files, and is not set up to write files to this format (due to complexity of writing the header, mlist, subheaders, and data blocks). Though I generally think most people will want to just import this file type and access the data.
I have added a very tiny (artificial) 2.1K tinypet.v file in the tests/data directory for running a subset of my tests.

Comments always welcome

--Cindee

[matthew-brett]

Sorry to come to this so late. I had only a quick look at the code, but it looks nicely and carefully done.

I suggest that I just merge, and have a look soon, with pull requests for you to look at, as they come to me.

The things that came to me at a very quick look are:

• You've used the binary header abstraction from analyze in the way that I hoped it would be useful, but there's no base class to inherit from, so you've implemented some stuff that is also in the analyze class. I should go back and restore the binaryheader class so you can inherit from it.
• There some emacs-looking whitespace at ends of lines, here and there
• You've used the ParametricTestCase thing a lot. I actually don't know if the problems here:

http://nipy.sourceforge.net/devel/guidelines/testing.html#many-tests-in-one-test-function

with the parametric decorator also apply to the ParametricTestCase, but if they do, then we should probably try and move away from it, by just using assert_true(True) without the yields.

[cindeem]

Hi Matthew..

Thanks for getting to this...

On Fri, Sep 9, 2011 at 1:10 PM, Matthew Brett <
reply@reply.github.com>wrote:

Sorry to come to this so late. I had only a quick look at the code, but it
looks nicely and carefully done.

I suggest that I just merge, and have a look soon, with pull requests for
you to look at, as they come to me.

Sounds good....

The things that came to me at a very quick look are:

• You've used the binary header abstraction from analyze in the way that I
  hoped it would be useful, but there's no base class to inherit from, so
  you've implemented some stuff that is also in the analyze class. I should
  go back and restore the binaryheader class so you can inherit from it.

True...its up to you...I could also find another solution to avoid Analyze
or override them...
Ill take a quick look

• There some emacs-looking whitespace at ends of lines, here and there

probably..ill double check and clean

• You've used the ParametricTestCase thing a lot. I actually don't know if
  the problems here:

http://nipy.sourceforge.net/devel/guidelines/testing.html#many-tests-in-one-test-function

with the parametric decorator also apply to the ParametricTestCase, but if
they do, then we should probably try and move away from it, by just using
assert_true(True) without the yields.

thats a reasonable point...I started this code a while back..Ill revisit
the tests with this in mind..

Reply to this email directly or view it on GitHub:
#46 (comment)

Cindee Madison
Jagust Lab
UC Berkeley
cindeem@gmail.com

[matthew-brett]

I'll have a go at fleshing out the binary header idea in another branch, will let you know.

In the meantime, I see that the doctests are failing - would you mind taking a look?

nosetests --with-doctest nibabel/nibabel/ecat.py

[matthew-brett]

You probably saw my planned refactoring in #50

Let me know if it makes ECAT harder.

I see that I used to use ParametricTestCase and parametric a lot, but I refactored them out gradually since your original fork. In fact those testing utilities don't exist any more in trunk... Sorry, I should have announced that more obviously a while ago.

[cindeem]

On Mon, Sep 12, 2011 at 11:17 AM, Matthew Brett <
reply@reply.github.com>wrote:

You probably saw my planned refactoring in
#50

Ill try to review....(might be a day or two if I can avoid my users)

Let me know if it makes ECAT harder.

I see that I used to use ParametricTestCase and parametric a lot, but I
refactored them out gradually since your original fork. In fact those
testing utilities don't exist any more in trunk... Sorry, I should have
announced that more obviously a while ago.

no worries...should be easy to clean up...again just give me a couple days

--C

Reply to this email directly or view it on GitHub:
#46 (comment)

Cindee Madison
Jagust Lab
UC Berkeley
cindeem@gmail.com

[matthew-brett]

Any interest in working on top of the wrapstruct changes?

[cindeem]

to work on top of wrapstruct changes....would you recommend rebasing on master?

and will add suggested change for windows-friendly file reference

[matthew-brett]

On Tue, Sep 27, 2011 at 2:44 PM, cindeem
reply@reply.github.com
wrote:

to work on top of wrapstruct changes....would you recommend rebasing on master?

Yes, if you can bear it. I was hoping that it would be useful for you
to let the ecat header structures inherit from wrapstruct - but I
might be deceiving myself. If so just ignore - and let me know.

The current trunk doesn't have the parametric tests anymore - but
you've probably already dealt with that.

and will add suggested change for windows-friendly file reference>

Thanks for that.

[cindeem]

On Wed, Sep 28, 2011 at 12:29 AM, Matthew Brett <
reply@reply.github.com>wrote:

On Tue, Sep 27, 2011 at 2:44 PM, cindeem
reply@reply.github.com
wrote:

to work on top of wrapstruct changes....would you recommend rebasing on
master?

Yes, if you can bear it. I was hoping that it would be useful for you
to let the ecat header structures inherit from wrapstruct - but I
might be deceiving myself. If so just ignore - and let me know.

Ill finish fixing parametric tests. and push that.
then Ill see if I can rebase, and use your updates (which does have its
appeal)
If it results in a fubar.......I might just have you pull ecats as it
is..then make a new fork to move into using the WrapStruct

im hoping to get to this today..but might be this weekend

The current trunk doesn't have the parametric tests anymore - but
you've probably already dealt with that.

and will add suggested change for windows-friendly file reference>

Thanks for that.

Reply to this email directly or view it on GitHub:
#46 (comment)

Cindee Madison
Jagust Lab
UC Berkeley
cindeem@gmail.com

[cindeem]

refactored to remove parametric tests. if you pull in this ecat, Ill for a new branch to work in the wrapstruct to avoid rebase issues.

thanks!

[matthew-brett]

OK - merging - thanks

[cindeem]

thanks..

Ill look at integrating your wrapstruct changes...try to get this updated in
a timely fashion
--C

On Wed, Oct 19, 2011 at 1:29 PM, Matthew Brett <
reply@reply.github.com>wrote:

OK - merging - thanks

Reply to this email directly or view it on GitHub:
#46 (comment)

Cindee Madison
Jagust Lab
UC Berkeley
cindeem@gmail.com

[matthew-brett]

On Wed, Oct 19, 2011 at 1:32 PM, cindeem
reply@reply.github.com
wrote:

thanks..

Ill look at integrating your wrapstruct changes...try to get this updated in
a timely fashion

Test failure on PPC - endian issues maybe?

http://nipy.bic.berkeley.edu/builders/nibabel-py2.6-osx-10.4/builds/16/steps/shell_1/logs/stdio

Do you have access to a big-endian machine somewhere for testing?
Otherwise I'll try and set you up with the machine in Giannini...

[matthew-brett]

On Wed, Oct 19, 2011 at 6:05 PM, Matthew Brett matthew.brett@gmail.com wrote:

On Wed, Oct 19, 2011 at 1:32 PM, cindeem
reply@reply.github.com
wrote:

thanks..

Ill look at integrating your wrapstruct changes...try to get this updated in
a timely fashion

Test failure on PPC - endian issues maybe?

http://nipy.bic.berkeley.edu/builders/nibabel-py2.6-osx-10.4/builds/16/steps/shell_1/logs/stdio

Do you have access to a big-endian machine somewhere for testing?
Otherwise I'll try and set you up with the machine in Giannini...

@cindeem - any progress? Can I help with a login?

[cindeem]

I sent an email a few days ago..
I do not have a big endian machine I can easily use, and would love a login
--c

On Tue, Oct 25, 2011 at 7:31 PM, Matthew Brett <
reply@reply.github.com>wrote:

On Wed, Oct 19, 2011 at 6:05 PM, Matthew Brett matthew.brett@gmail.com
wrote:

On Wed, Oct 19, 2011 at 1:32 PM, cindeem
reply@reply.github.com
wrote:

thanks..

Ill look at integrating your wrapstruct changes...try to get this
updated in
a timely fashion

Test failure on PPC - endian issues maybe?

http://nipy.bic.berkeley.edu/builders/nibabel-py2.6-osx-10.4/builds/16/steps/shell_1/logs/stdio

Do you have access to a big-endian machine somewhere for testing?
Otherwise I'll try and set you up with the machine in Giannini...

@cindeem - any progress? Can I help with a login?

Reply to this email directly or view it on GitHub:
#46 (comment)

Cindee Madison
Jagust Lab
UC Berkeley
cindeem@gmail.com

[matthew-brett]

Huh - I didn't get it. I'll make you a login tomorrow. Can you send me your public ssh key somehow?

[cindeem]

On Tue, Oct 25, 2011 at 8:59 PM, Matthew Brett <
reply@reply.github.com>wrote:

Huh - I didn't get it.

it is odd...but no worries

I'll make you a login tomorrow. Can you send me your public ssh key
somehow?

Ill send you one tmr..
thanks

C

Reply to this email directly or view it on GitHub:
#46 (comment)

Cindee Madison
Jagust Lab
UC Berkeley
cindeem@gmail.com

[cindeem]

Here is my public key,

or if you have access to the BIC raid, we could set up a passwd (tmp) and I
could change...
If you see Julie you can ask her how we do this..

--C

On Tue, Oct 25, 2011 at 9:02 PM, Cindee Madison cindee@berkeley.edu wrote:

On Tue, Oct 25, 2011 at 8:59 PM, Matthew Brett <
reply@reply.github.com>wrote:

Huh - I didn't get it.

it is odd...but no worries

I'll make you a login tomorrow. Can you send me your public ssh key
somehow?

Ill send you one tmr..
thanks

C

Reply to this email directly or view it on GitHub:
#46 (comment)

Cindee Madison
Jagust Lab
UC Berkeley
cindeem@gmail.com

Cindee Madison
Jagust Lab
UC Berkeley
cindeem@gmail.com

[matthew-brett]

Any progress on this one? I'm hoping to do a release fairly soon.

[cindeem]

I fixed this a while back and did a new pull request....sorry our communication seems to not be working...let me know if you cannot find the pull request

[matthew-brett]

On Fri, Nov 4, 2011 at 3:25 PM, cindeem
reply@reply.github.com
wrote:

I fixed this a while back and did a new pull request....sorry our communication seems to not be working...let me know if you cannot find the pull request

Should it be here:

https://github.com/nipy/nibabel/pulls

?

[cindeem]

sorry, not sure why its not there...
Ill see if I can get a proper pull request, hopefully tonight

On Fri, Nov 4, 2011 at 3:30 PM, Matthew Brett <
reply@reply.github.com>wrote:

On Fri, Nov 4, 2011 at 3:25 PM, cindeem
reply@reply.github.com
wrote:

I fixed this a while back and did a new pull request....sorry our
communication seems to not be working...let me know if you cannot find the
pull request

Should it be here:

https://github.com/nipy/nibabel/pulls

?

---

Reply to this email directly or view it on GitHub:
#46 (comment)

Cindee Madison
Jagust Lab
UC Berkeley
cindeem@gmail.com

[matthew-brett]

On Fri, Nov 4, 2011 at 3:49 PM, cindeem
reply@reply.github.com
wrote:

sorry, not sure why its not there...
Ill see if I can get a proper pull request, hopefully tonight

Thanks - that would be very helpful.