This is potentially ready to merge. All of the converters' junit tests are now running and passing.

@awalter17 are there plans to implement converters from e.g. int[] to long[], short[] to long[], float[] to double[] etc?

are there plans to implement converters from e.g. int[] to long[], short[] to long[], float[] to double[] etc?

The only viable general solution is to be recursive. The plan is to split out the collections logic of the DefaultConvertService to support arrays and Collections element by element. That will handle e.g. int[] to long[] as well as int[][] to long[][] etc.

This will be part of #109. See #109 (comment)

Actually, couldn't I just write an adapter from e.g. int[] to long[] with the current framework? Or are you saying that this is not really a nice / generic way? Sorry, maybe I'm not getting your point here.

@dietzc You could write such adapters, but you would need many of them. And they wouldn't handle e.g. int[][] to long[][]. Or int[][][] to long[][][]. Or... etc. And they will all be obsolete once we have the improved collections support.

After testing this PR a bit it looks like it needs some more work.

Since all of the new converters are the same priority as the DefaultConverter, they are not actually picked, and thus are not being tested when going through the ConvertService - the DefaultConverter wins. A quick fix would be to set these at slightly higher priority than the DefaultConverter but my preference would be to see the DefaultConverter gutted so it doesn't do primitive conversions any more. Any functionality it retains that might use primitive conversions (e.g. primitive array conversions..? I don't remember) would just use the ConvertService and thus these primitive converters.

Also, it seems like true primitive types (e.g. 'int', 'float') are not actually supported. We should not have dedicated converters for these, but ensure the getNonprimitiveClass method is being used in the converter.supports calls for all primitive converts, so they will properly claim these conversion requests.

A simple groovy script demonstrating these problems:

// @org.scijava.convert.ConvertService cs

println cs.getHandler(Integer, Double) // Case 1. This should NOT be DefaultConverter
println cs.getHandler(int, double) // Case 2. This should come back IntegertoDoubleConverter

If the functionality is stripped out of DefaultConverter, case 1 does not need explicit tests. Case 2 should have unit tests, however.

Indeed, removing primitive conversion from the DefaultConverter was supposed to be part of this work.

case 1 does not need explicit tests.

Why not? Would be good to ensure behavior doesn't break in the future. I would prefer a unit test of it.

@ctrueden OK so my original tests were flawed because it was testing PyInt and PyFloat instead of java native classes. Updated to a groovy script. As a side note, the DefaultConverter does properly support primitive conversion (as we inferred from looking at the unit tests)

case 1 does not need explicit tests.

Why not?

I meant that we may not want the unit tests to rely on particular concrete classes coming back as handlers, but we do want to verify the convert service can handle primitive conversions. Or at least these tests shouldn't be conflated (i.e. I think a test for for ConvertService is the most important.. but maybe a test for primitive converters that has overlap but uses concrete converters would be useful..?)

OK I just added 2 commits that allow the new primitive converters to handle unboxed primitive conversion requests. Currently they still are not because of their priority relative to the DefaultConverter.. but as the DefaultConverter is gutted primitive conversion should remain functional.

Just want to point out a use case where having the CastingConverter might be really nice: imagej/imagej-ops@da4b24e. Because as things stand, that code actually ignores generic types for the most part, so e.g. an ArrayList argument (which was written as ArrayList<String> and contains only String objects) would be matched with a Collection<? extends Class<?>> since the Type describing the latter boils down to a raw Collection and nothing more...

Rebased over latest master, and all tests pass. So let's merge this long overdue branch.