scijava / scijava-common

A plugin framework and application container with built-in extensibility mechanism :electric_plug:
BSD 2-Clause "Simplified" License
87 stars 52 forks source link

Add Multiple Select parameter #260

Open imagejan opened 7 years ago

imagejan commented 7 years ago

It would be useful to have a Multiple Select parameter in addition to the single choice that is possible with String parameters and the choices annotation.

Maybe this could be implemented as a new SelectionWidget with two styles:

What do you think?

ctrueden commented 7 years ago

@imagejan What would you suggest for the underlying data structure? I.e.: the actual type of the parameter?

imagejan commented 7 years ago

Maybe String[] or List<String>?

ctrueden commented 7 years ago

I took a look, but this is a bit tricky type-wise.

The ModuleItem interface defines the following API:

/** Gets the list of possible values. */
List<T> getChoices();

/** Gets the item's current value with respect to the given module. */
T getValue(Module module);

/** Sets the item's current value with respect to the given module. */
void setValue(Module module, T value);

Suppose we have a parameter like this:

@Parameter(choices = {"Alice", "Bob", "Charlie"})
List<String> names;

The type T of the parameter is List<String>, which makes sense for getValue and setValue, but getChoices in this case returns List<List<T>> so we would have to—I guess—return each name in its own singleton list. I do not believe such a conversion currently happens. And it is a bit misleading since the idea of "choices" now changes: it does not imply the value must equal one and only one of the choices given anymore.

But some hackish solution still seems feasible on the surface, so I started trying to implement something. But it does not work yet.

And here is the Groovy script I used for testing:

// @String(choices = {"quick", "brown", "fox"}) single
// @java.util.List(choices = {"the", "lazy", "dog"}) multiple
println("single = " + single)
println("multiple = " + multiple)
imagejan commented 7 years ago

But it does not work yet.

Right, I get:

java.lang.NullPointerException
    at org.scijava.widget.DefaultWidgetModel.getChoices(DefaultWidgetModel.java:234)

As far as I get it, choicesList.get(i) seems to be null here: https://github.com/scijava/scijava-common/blob/f7457f0e0538a9f61b25ffe43988660766e14a7a/src/main/java/org/scijava/widget/DefaultWidgetModel.java#L234

... so item.getChoices() seems to return a list of null values ?!

ctrueden commented 7 years ago

so item.getChoices() seems to return a list of null values

Yep.

Upon further thought, I think changing the implicit contract for getChoices() is not the way to go here. That method is supposed to return a list of constrained values for the item, and I don't want to pervert that here.

Instead, we should expand the ModuleItem API to add new method(s) with something like getPossibleElements() that returns, I dunno, I guess just List<Object>, since there is no way to decompose the T here. Alternately we could make a new ModuleListItem<E> extends ModuleItem<List<E>> although that comes with its own challenges.

This project is, unfortunately, too involved for me to tackle any time soon. It requires some experimentation, together with careful thought about the API and implications.

imagesc-bot commented 4 years ago

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

https://forum.image.sc/t/how-to-customize-override-scijava-ui/43642/4