Code review comment for lp:~nipy-developers/nipy/trunk-mb

Hi,

Thanks very much for the review, it's very good to have some feedback
on the code.

On Wed, Jan 13, 2010 at 4:51 PM, Fernando Perez <email address hidden> wrote:
> Very minor nits
>
> 27      +from nipy.core.reference.coordinate_map import CoordinateMap, Affine, compose, \
> 28      + drop_io_dim, append_io_dim
>
> I'd use
>
> from ... import (...
> ...)
>
> now that this syntax is allowed.

Yes, we should move that way now, I agree.

> 129     +@parametric
> 130     +def test_array_coord_map():
>
> Maybe a few comments?  If one of those tests ever breaks, someone who understands the code less could be in a tight spot to fix them back.

Yes, fair point.

> In:
>
> 345     +def load(filename, *args, **kwargs):
> 346     + ''' Load file given filename, guessing at file type
> 347
> 348     Parameters
> 349     ----------
> 350     - filespec : string or file-like
> 351     + filename : string or file-like
>
> I actualy found 'filespec' to be clearer, because when I see 'filename' I immediately think "it's already a string", where as filespec to me conveys the idea of name-or-open-handle better.

Ah - yes - actually it's much worse than that. In your Ipython
coding frenzy you may have missed that:

img = Spm99AnalyzeImage(arr, aff)
img.to_filename('test')

actually saves to 'test.img', 'test.hdr', and 'test.mat'. I think
that filespec is better too, but Gael was finding the name a little
confusing, because you usually use it just with a filename.

See you,

Matthew