OpenImageIO

Stéfan van der Walt stefan at sun.ac.za
Sat Oct 10 02:12:04 EDT 2009


Hey Ralf

2009/10/9 Ralf Gommers <ralf.gommers at googlemail.com>:
> http://github.com/rgommers/scikits.image/blob/imgcollection/scikits/image/io/io.py

Thanks for working on this!

> Questions:
> - do you want to keep the Image class in that form? It seems either a plain
> ndarray or ndarray + tags dict is enough.

I'd like to remove the image class entirely.

> - can I remove the EXIF stuff or move it to a subclass of Image? I don't
> think it belongs in the base Image class.

Yes, although having an EXIF reader as a separate function might be
handy!  That code is BSD-licensed AFAIK.

> - should imread be moved into io.py?

Let's leave it where it is for now.  It is accessible as
scikits.image.io.imread, which is fine from a user perspective.

Other notes:

If you require PIL trunk, you need to check that it is available
explicitly.  Also, the PIL import test is already done by imread.

About naming: I'd prefer if we expand the names, i.e. MultiImage
instead of MultiImg.  I've learnt this the hard way, but it seems I
can never remember my own shorthand :)

The example markup does not require the "::".

The description of MultiImage could be changed to reflect what it is
storing, i.e. something like

class MultiImage(object):
   """Class for loading multi-layer images."""

When using try-except statements, keep the code snippet contained as
small as possible.  In this case, there's no problem really, because
you specifically wait for an EOFError.  In general, however, it's
safer to use the form:

i = 0
while True:
    i += 1
    try:
        img.seek(i)
    except EOFError:
        break
return i

Not sure whether you'll ever come across images without any frames
inside, but in those cases you need a return statement as well, as
above.

_getallframes can be simplified using _getframe:

frames = []
for i in range(len(self)):
    frames.append(self._getframe(i))
return frames

The numframes variable should not be exposed, since len() is already available.

The string representation can also include information on the number
of frames, e.g.

cat.tiff [50 frames]

Finally, ensure that read-only members are defined as properties:

@property
def filename(self):
    return _filename

As always with review comments: they may be overly pedantic, so use
what you find applicable and discard the rest.

Cheers
Stéfan



More information about the scikit-image mailing list