imagej / imagej-legacy

ImageJ+ImageJ2 compatibility layer
https://imagej.net/libs/imagej-legacy
BSD 2-Clause "Simplified" License
16 stars 25 forks source link

Add test for String conversion to images #261

Closed imagejan closed 2 years ago

imagejan commented 3 years ago

We should ensure that String values are converted to ImagePlus (or Dataset) objects only if there actually is an already opened image matching the conversion request.

If there is no matching image, we shouldn't try to convert (and maybe return null), so that other converters can take over and load e.g. from a file path.

imagejan commented 3 years ago

What I did so far in this pull request:

Regarding the last point, I checked which class is returned when calling convertService.getHandler(nonExistentTitle, Dataset.class), and (unsurprisingly) we get io.scif.convert.StringToDatasetConverter (see also https://github.com/scifio/scifio/pull/443). But I was astonished to see that despite the SCIFIO converter kicking in here, the correct opened image seems to be provided nevertheless.

See also https://github.com/imagej/imagej-legacy/issues/246 for related discussion.

maarzt commented 2 years ago

This PR no longer works, due to changes in SCIFIO. There's also still a bug in StringToImagePlusConverter.convert(...) method. It doesn't check the destination type to be ImagePlus.

Closed in favor of #272

(@imagejan Sorry! I create a nearly identical PR before finding this one. But thanks to your PR, I found the but in the StringToImagePlusConverter, which would have otherwise caused some trouble.)