Clean up NativeType creators

[hinerm]

@dietzc I didn't do a great job reviewing the img-creators branch. After it broke Jenkins I fixed DefaultCreateNativeType to hard-code the DoubleType return, which made it compatible with J6 again.

However, upon looking closer there are now some questions about this implementation:

• Do you really want the DefaultCreateNativeType to only return DoubleTypes, or did you want a generic parameterized implementation?
• DefaultCreateImg implements CreateNativeImg but if not given an output type, calls CreateType - should it call CreateNativeType?
• Also, in DefaultCreateImg you can not assume that calling CreateType with no args will give you back a T. I started a branch with a unit test demonstrating this where you get back a DoubleType image after providing a ShortType factory, which I think is counter-intuitive.

When you get a chance could you address these topics on the branch I started, and file a PR?

[dietzc]

Hi @hinerm,

DefaultCreateImg implements CreateNativeImg but if not given an output type, calls CreateType - should it call CreateNativeType?

Thanks. I will fix that

Also, in DefaultCreateImg you can not assume that calling CreateType with no args will give you back a T. I started a branch with a unit test demonstrating this where you get back a DoubleType image after providing a ShortType factory, which I think is counter-intuitive.

I see the problem. But I'm not sure how we can fix that. We can add a check if the provided factory has the same type as output type, but how would we resolve this issue if the types are different? Throw an exception?

Do you really want the DefaultCreateNativeType to only return DoubleTypes, or did you want a generic parameterized implementation?

I'm not sure if I understand. A parametrized implementation would expect a Class of type T as input to create some Type of type T? The point with DefaultCreateNativeType is that, if nobody overrides it with another, higher priority op, it gives you the most general NativeType. If I parametrize the Op the user has to provide the type again?

[dietzc]

DefaultCreateImg implements CreateNativeImg but if not given an output type, calls CreateType - should it call CreateNativeType?

df2f463

Also, in DefaultCreateImg you can not assume that calling CreateType with no args will give you back a T. I started a branch with a unit test demonstrating this where you get back a DoubleType image after providing a ShortType factory, which I think is counter-intuitive.

see: 7dba713

[hinerm]

I see the problem. But I'm not sure how we can fix that. We can add a check if the provided factory >has the same type as output type, but how would we resolve this issue if the types are different? >Throw an exception?

@ctrueden and I talked about this a bit and decided it is really a limitation of the ImgLib2 library putting the generic parameter of ImgFactory on the class, instead of the constructor or create methods.

A parametrized implementation would expect a Class of type T as input to create some Type of type T? >The point with DefaultCreateNativeType is that, if nobody overrides it with another, higher priority op, it >gives you the most general NativeType. If I parametrize the Op the user has to provide the type again?

Correct. A generic parameter implies that DefaultCreateNativeType is able to generate any NativeType. But that would only be possible if the op had an input type parameter - and it seems like the purpose is to basically declare a consistent default NativeType to be used in the Ops framework, because if you have a NativeType to pass to CreateNativeType then you already have the type instance you want. So I'm pretty sure just removing the generic parameter was the correct thing to do.

I think what we want is to patch ImgLib2 to add constructors to ImgFactory impls that accept a type. The CreateImgFactory op - whose purpose, I believe, is just to decide which factory (planar, array, cell, etc...) to create - would then also be stripped of its generic parameter, instead just having an optional NativeType parameter.

[dietzc]

agreed to all points 👍

[dietzc]

should we leave this issue open until ImgFactory in imglib2 is adapted? because this blocks m1.

[ctrueden]

should we leave this issue open until ImgFactory in imglib2 is adapted?

Sure.

because this blocks m1.

Fortunately, m1 is a synthetic milestone that cannot really be "blocked" per se. It never actually closes. Rather, as issues get completed, they get reclassified to their actual milestone releases that first included them. See http://imagej.net/Issues#What_are_issues_for.3F for details.