Add displayName and display options to Params (See #10474)

[joshmoore]

These were initially suggested by Curtis during work
on imagej-omero in order to allow storing the same
info that's available in IJ2 plugins as params. No
GUI code is currently consuming the fields, but if
they are present for 5.1.0, then further PRs can.

cc: @ctrueden @jburel @gusferguson @will-moore @knabar

[will-moore]

Looks good. This reminds me that @jburel and I previously discussed a flag for scripts to indicate that they shouldn't show up in the scripts menu. E.g. populate_rois, or all the various Figure scripts.

I just noticed that scripts UI is broken on trout (almost certainly not due to this PR!)..

[joshmoore]

@will-moore : would "bool display" get you that? Or you would need that in the DB?

[will-moore]

@joshmoore Sounds good. Presumably this would be set in the script declaration?

client = scripts.client("My_script", "is great", display=False)

[joshmoore]

Yes. With this PR in, we could try to update one of the existing scripts to make use of display, and see if that gets us anywhere.

[will-moore]

@joshmoore Yes, would be good to try it with Figure scripts.

It looks like the currently broken script parameters is not just in web, but it's working OK in develop. @joshmoore @jburel Any ideas which PR may be causing this?

[ctrueden]

Thanks, @joshmoore.

[will-moore]

@joshmoore Were you going to add script parameter for bool: display in this PR?

[joshmoore]

You mean https://github.com/openmicroscopy/openmicroscopy/pull/3562/files#diff-7d2d1dbcbade62f3037c81279a2b344eR296 ?

[jburel]

@will-moore's comment is more if we can hide the script itself not only one parameter

[will-moore]

@joshmoore No - it appears to me that that commit is about hiding a parameter. I'm asking about hiding a script.

[joshmoore]

Ah, understood. I'll look into it. Thanks.

[joshmoore]

So on this front: at the moment at least, you don't grab the scripts, only the file paths, correct?

[will-moore]

I actually grab the scripts to get the paths etc, so could easily test for 'display' at this point.
https://github.com/openmicroscopy/openmicroscopy/blob/develop/components/tools/OmeroWeb/omeroweb/webclient/views.py#L2760

[joshmoore]

I've added the field, @will-moore, but you'd have to individually load params() for every script. This is likely going to be time consuming. Like the changes to IConfig it might be best to have a method which returns both the script object as well as the params for all scripts not filter (whether via display or via bin/omero config).

[will-moore]

Ah, OK - understood.

[joshmoore]

This is leading to marshaling exceptions (with hard to track down UI problems). Closing for the moment. Minimally we would need to invalidate all the params that are stored in the DB.

[ctrueden]

The SciJava module framework's @Parameter fields (i.e., metadata fields for script parameters) are here:

• https://github.com/scijava/scijava-common/blob/scijava-common-2.38.2/src/main/java/org/scijava/plugin/Parameter.java#L55-L165

And the @Plugin fields (i.e., metadata fields for scripts themselves) are here:

• https://github.com/scijava/scijava-common/blob/scijava-common-2.38.2/src/main/java/org/scijava/plugin/Plugin.java#L68-L189

Here is the current thing that converts from ModuleItem (SciJava) to omero.grid.Param:

• https://github.com/imagej/imagej-omero/blob/imagej-omero-0.2.1/src/main/java/net/imagej/omero/DefaultOMEROService.java#L98-L114

So we already have mappings for type, description, required/optional, multiple choice, min and max.

Relevant parameter-level fields that SciJava has which OMERO scripting does not yet have:

• style - this lets you give a hint about the UI widget to use for harvesting the parameter value. E.g., for numbers, the SciJava Swing UI library supports spinner, slider and scroll bar widgets. For multiple choice it supports combo boxes and radio button groups.
• visibility - for data provenance; probably easiest if you just read the javadoc.
• stepSize - goes along with min and max, as another quick constraint on numeric inputs (without needing to resort to a callback; see below)
• callback - for fields whose values are constrained in a custom way; e.g., dynamically affected by the values of other fields. So e.g. you can have numeric fields width and height, and then a boolean checkbox constrainAspectRatio, which when checked causes the height to automatically adjust when the user edits the width and vice versa. Requires client-side code to implement though; might be too involved for the OMERO scripts use case.
• initializer - for fields whose initial values are computed in some custom way; e.g., dynamically computed from the initial values of other fields. Also might be more complex than OMERO scripts want.
• columns - a hint for how wide to make a UI text field. In retrospect I think it was unnecessary and IIRC unused, and will be removed in SJC3, so let's definitely not propagate this.
• attrs - a list of additional freeform key/value pairs, basically like OME's map structured annotation, but for module parameters, as well as for modules themselves. We use it in a couple of ways to extend the parameter model for specific scenarios. E.g., the OPS framework uses it to add function name aliases for an op. And e.g., the ImageJ legacy UI uses it to flag whether a module should be available from the ImageJ legacy UI or not.
• persist, persistKey - for saving and loading last-used values via the Java preferences API, so they survive across JVM restarts. I am guessing this behavior would not be desired for OMERO scripts. It actually causes some problems for scientific reproducibility, although users do find it convenient. We are still struggling to strike a balance there.
• autoFill - used in conjunction with the preprocessing framework; some preprocessors will take the liberty of filling in certain kinds of inputs with the presumed desired value. E.g., when an ImageJ UI is visible, there are a few "single input" preprocessors that do things like populate a single image input with the currently active image (i.e., focused image window). But this concept is probably inapplicable to OMERO scripts.

Relevant module-level fields that SciJava has which OMERO scripting does not yet have:

• menu, menuRoot - a way to define where the module belongs in the menu structure. Right now OMERO uses the nested directory structure to achieve that, which is probably OK as long as you don't need any weird characters in your menu titles.
• iconPath - a way to give a module a cute little icon (for use in the menus, or otherwise). You guys might want to have this.
• priority - a way to override a module with another module: you give it the same name and/or menu path, but at a higher priority, and it will take precedence. Probably not needed for OMERO, at least not in the near term. (It really depends how "distributed" script collections become in the future, I think. If you start having OMERO script "update sites", it could be valuable.)
• selectable, selectionGroup - for modules whose menu entry is a check box or a radio box. In practice, this feature's design is currently "not quite right" conceptually, so I suggest ignoring it until we have time to iterate it further.
• headless - Well, all OMERO scripts are headless, so this flag is irrelevant there.
• initializer - a way to initialize all the parameter fields globally; slightly redundant with the initializer of each parameter, but sometimes one make sense conceptually over the other, depending on the algorithm.
• enabled - whether the module should be grayed out in the UI.
• visible - whether the module should be visible in the UI.

OK, I just saw that you closed this, so I'll stop writing a novel since we might not have the luxury of adding any fields in the near future...

[joshmoore]

Not at all, @ctrueden. We're just trying a new schema of more proactively closing a PR when it's causing problems. This will be re-opened. I just have to find a good way to invalidate the script cache.

[ctrueden]

OK, sounds good. Let me know if you have any questions about any of the above, then.

[joshmoore]

Pushed improved error handling.

[joshmoore]

@ctrueden : from reading your novel, is an appropriate summary of things to consider (initially) attrs and icon?

[ctrueden]

Recommend:

• For parameters:
  • style (string / extensible enum)
  • visibility (enum)
  • stepSize (double)
  • attrs (string/string map)
• For scripts:
  • menu/menuPath (string, or list of string) - solves the same naming problems as labels, but at non-leaf levels
  • iconPath (file path)
  • visible (boolean)
  • attrs (string/string map)

Consider:

• For scripts:
  • priority (double) - for future extensibility
  • enabled (boolean)

[pwalczysko]

Before merging, I would recommend to double check that
trout / merge, user-4 read-only-1, any image (.dv for example), Script Channel Offsets from Utility scripts works fine. Although the problem might have to be dealt with elsewhere, it would be good to have this double-security here. (it works fine as I am writing it)

[jburel]

• menu/menuPath (string, or list of string). This probably more important for ij plugins since we are using the directory structure to build the menu path.

[will-moore]

@pwalczysko I can confirm that scripts are working OK in trout.

[will-moore]

@jburel - I could imagine a situation where you might want the menu hierarchy to be organised differently or named differently than the actual file path. But, as discussed above re: script display, we only have the file path available when loading the script menu, since we don't want to load every script at this point. So, not ready to support this yet.
NB: I also noticed that @ctrueden suggested 'visible' attribute for scripts and parameters instead of 'display'. Don't know if anyone has a strong preference either way?

[mtbc]

I suppose it fits CSS somewhat: "visible" about if to display, "display" about how to display?

[jburel]

I think we should not rush for 5.1, since we still need to discuss parameters to expose. I will close it for now and re-open during the 5.1.x or 5.2

[joshmoore]

I'm not sure it can be opened for 5.1.x; at least, we'll need to put some thought into whether or not it will cause any problems. (Need a cross-version test)

[jburel]

Due to the busy schedule, 5.2 is probably more realistic

[joshmoore]

Closing again for the moment then. Trello cards all appropriately filed.