Skip to content

Fix duplicate entries in input selectors#482

Merged
hinerm merged 1 commit into
masterfrom
limit-widget-inputs
Sep 13, 2024
Merged

Fix duplicate entries in input selectors#482
hinerm merged 1 commit into
masterfrom
limit-widget-inputs

Conversation

@hinerm

@hinerm hinerm commented Sep 6, 2024

Copy link
Copy Markdown
Member

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

hinerm commented Sep 10, 2024

Copy link
Copy Markdown
Member Author

(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
De-duplicate widget inputs by String comparison
f5230f6 
When populating potential items for an input widget, we now prefer
existing objects of a given input type to potentially convertiable
types.

Further, for convertibles we now avoid considering them if they share a
toString with any other potential input. Candidates are prioritized in
order returned by ConvertService.getCompatibleInputs.

This mitigates the potential for duplicate entries in input harvesters
when multiple input instances are convertible to the same effective
output instance.
@hinerm hinerm force-pushed the limit-widget-inputs branch from f5b020d to f5230f6 Compare September 10, 2024 20:12
@hinerm

hinerm commented Sep 10, 2024

Copy link
Copy Markdown
Member Author

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 hinerm marked this pull request as ready for review September 12, 2024 17:19
@hinerm

hinerm commented Sep 12, 2024

Copy link
Copy Markdown
Member Author

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

@hinerm hinerm mentioned this pull request Sep 12, 2024
Closed
@hinerm hinerm merged commit ee1e7c1 into master Sep 13, 2024
@hinerm hinerm deleted the limit-widget-inputs branch September 13, 2024 18:56
@imagesc-bot

imagesc-bot commented Jan 22, 2025

Copy link
Copy Markdown

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hinerm @imagesc-bot