IterableMax.compute( Iterable ) always returns DoubleType regardless of input generic parametr

[hanslovsky]

This is a follow-up to a bug report on the ImageJ forum and mostly c&p of my post in that thread:

MWE

import net.imagej.ImageJ;
import net.imglib2.img.array.ArrayImg;
import net.imglib2.img.array.ArrayImgs;
import net.imglib2.img.basictypeaccess.array.FloatArray;
import net.imglib2.type.numeric.real.DoubleType;
import net.imglib2.type.numeric.real.FloatType;

public class Dummy
{

	public static void main( final String[] args )
	{
		final ImageJ ij = new ImageJ();
		final ArrayImg< FloatType, FloatArray > floats = ArrayImgs.floats( 1, 2, 3 );
		final DoubleType doubleResult = ( DoubleType ) ( Object ) ij.op().stats().max( floats );
		final FloatType floatResult = ij.op().stats().max( floats );
		System.out.println( doubleResult + " " + floatResult );
		ij.context().dispose();
	}

throws this exception:

Exception in thread "main" java.lang.ClassCastException: net.imglib2.type.numeric.real.DoubleType cannot be cast to net.imglib2.type.numeric.real.FloatType
	at Dummy.main(Dummy.java:16)

As you can see, that weird ( DoubleType ) ( Object ) cast works in line 15, but the expected correct line 16

final FloatType floatResult = ij.op().stats().max( floats );

throws an exception.

I am not an imagej-ops expert by any means but here are my 2 cents: The culprit is probably AbstractStatsOp.createOutput: It always creates a DoubleType. IterableMax could override createOutput, e.g. something like this:

@Override
public T createOutput( final Iterable< T > input )
{
	return input.iterator().next().createVariable();
}

The problem here is that this will throw an exception if input has zero elements.

[bnorthan]

I just created a branch that implements @hanslovsky solution. If people think this is an acceptable solution I could also modify the min, and median op and open a pull request for review.

[imagejan]

@bnorthan wrote:

I could also modify the min, and median op

Only the min and max ops should be changed accordingly. For the median, it makes sense to return a different output type, as the median can be e.g. 1.5 for [1, 2].

This is in line with the method signatures in the StatsNamespace:

<T extends RealType<T>>T                       | max(Iterable<T> in)
<T extends RealType<T>>T                       | max(T out,    Iterable<T> in)
<T extends RealType<T>,O extends RealType<O>>O | mean(Iterable<T> in)
<T extends RealType<T>,O extends RealType<O>>O | mean(O out,     Iterable<T> in)
<T extends RealType<T>,O extends RealType<O>>O | median(Iterable<T> in)
<T extends RealType<T>,O extends RealType<O>>O | median(O out,       Iterable<T> in)
<T extends RealType<T>>T                       | min(Iterable<T> in)
<T extends RealType<T>>T                       | min(T out,    Iterable<T> in)

[bnorthan]

Good point @hanslovsky. I just opened pull request #560.

[hanslovsky]

Just to make sure that I am not taking away someone else's credit: I am sure @bnorthan meant to mention @imagejan

@imagejan Shouldn't the signature of

<T extends RealType<T>,O extends RealType<O>>O median(Iterable<T> in)

be

<T extends RealType<T>> DoubleType median(Iterable<T> in)

instead? After all, we always return DoubleType, no? Same for mean.

[bnorthan]

Yes I meant @imagejan. Thanks @imagejan.

[imagejan]

Thanks to both of you for working on this! 😄

After all, we always return DoubleType, no?

The namespace defines the contract, and there could possibly be ops implementations in the future that return a RealType other than DoubleType that better fits to the given input type, right?

But that's really up to the ops architects (@imagej/ops-admin).