[ImageJ-devel] [fiji-devel] New methods in CalibratedSpace

[Stephan Saalfeld]

Hi,

I would not change the way ImgPlusImg and ImgPlus are designed right now
and not mix them.  Keep in mind that ImgPlusImg can return the ImagePlus
by which you have access to Calibration and all the other ImageJ
metadata.

I support adding a method wrapImgPlus or wrapToImgPlus (for better
grammar).

Best,
Stephan




On Thu, 2012-07-12 at 17:40 -0400, Stephan Preibisch wrote: 
> Good point J-Y, wrap should always return an ImagePlusImg ... 
> 
> But I do not really like that ImagePlusImg is an ImgPlus, because ImgPlus is an Img and has an Img. And on top ImagePlusImg is an Img, too. But then it could return itself, no, that gets too convolved for my taste ...
> 
> why not add a second wrap method then, we could call it:
> 
> ImgPlus imp = wrapMetaData( ImagePlus );
> 
> or so?
> 
> On Jul 12, 2012, at 17:28 , Jean-Yves Tinevez wrote:
> 
> > On 12-Jul-12 23:22, Curtis Rueden wrote:
> >> Hi Steffi & J-Y,
> >> 
> >> > if ( img instanceOf ImagePlusImg )
> >> 
> >> I think using instanceof in this way is quite evil, and should be avoided whenever possible. It is something I very much want to avoid in the core ImgLib2 and ImageJ2 code, because it prohibits extensibility. Better is to put the common functionality into an interface method declaration and then implement it differently in the different subclasses.
> >> 
> >> That said, how to avoid writing code like this depends on the situation. We'd have to look over the various places in code that it is currently happening...
> >> 
> >> > 1) break it and maybe it is just fine
> >> > 2) make a wrapImgPlus method
> >> > Maybe 1) is the better way to go, I just wanted to bring up potential implications ....
> >> 
> >> In this case, given that it would be easy to simply add the wrapImgPlus methods, that may be a safer way to go. Anyone else have an opinion?
> > 
> > Maybe
> > 
> > we could take the thing in reverse:
> > 
> > - have wrap() methods return an actual ImagePlusImg
> > - have ImagePlusImg be an ImgPlus
> > 
> > I quickly looked and this is not trivial: ImagePlusImg is already a PlanarImg, and we would have to add some hierarchy in the middle...
>