scijava-common icon indicating copy to clipboard operation
scijava-common copied to clipboard

Fix duplicate entries in input selectors

Open hinerm opened this issue 1 year ago • 3 comments

This PR represents an alternative to this PR attempting to fix the duplicates earlier in the framework.

However, this does create a problem if the converters themselves consistently create new items. For example, this is what happens when running a trivial script multiple times:

//@ImagePlus in1
//@ImagePlus in2

println(in1)
println(in2)

image

This could be a bug in particular converters (ImageTitleToImagePlusConverter is a likely suspect) but it does raise a concern of potential behavior for other converters, too.

hinerm avatar Sep 06 '24 19:09 hinerm

(ImageTitleToImagePlusConverter is a likely suspect)

OK obviously it wasn't the title converter but DatasetToImagePlusConverter which registers Datasets blindly without checking for existing mappings.

A simple fix. We also decided to just not aggressively convert items though, and instead de-duplicate on Strings

hinerm avatar Sep 10 '24 18:09 hinerm

In conjunction with these imagej-legacy changes this branch now appears to resolve the Fiji input harvester bug.

However, the String de-duplication is of course fragile, and does not address one of the goals in that inputs will change after the first run. The only way I'm aware of to satisfy the goal of consistent outputs would be to aggressively convert input candidates. That option is also working with the aforementioned imagej-legacy branch.

So we have to pick one: a) String comparison b) Aggressive conversion

Functionally I like b more but I am worried it will open a whole can of worms in the future as we discover more badly-behaving converters, whereas a may prove insufficient in some cases but works in the current obvious, annoying case, and could be expanded by tapping into Named, etc.

hinerm avatar Sep 10 '24 20:09 hinerm

OK, decided that this is the least intrusive change (compared to converting) and we can fix the ImagePlus title elsewhere if needed

hinerm avatar Sep 12 '24 17:09 hinerm

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/results-of-colocalization-by-cross-correlation-ccc-plugin-failed-to-match-actual-image-conditions/105423/5

imagesc-bot avatar Jan 22 '25 05:01 imagesc-bot