aces / cbrain

CBRAIN is a flexible Ruby on Rails framework for accessing and processing of large data on high-performance computing infrastructures.
GNU General Public License v3.0
71 stars 42 forks source link

Boutiques integration not working for with list=true and a select set of options #848

Closed shots47s closed 4 years ago

shots47s commented 5 years ago

The following Boutiques descriptor implement fails to produce a useful UI element in CBRAIN:

       {
            "command-line-flag": "--steps",
            "description": "Longitudinal pipeline steps to run.",
            "id": "steps",
            "list": true,
            "name": "steps",
            "optional": true,
            "type": "String",
            "value-choices": [
                "cross-sectional",
                "template",
                "longitudinal"
            ],
            "value-key": "[STEPS]"
        },

This should produce a list with each element being a select box to set. Right now one only gets a single select box and then if it is set, the task receives a single value and fails to validate against the schema.

image

Fails in the task with the following error:

[2019-08-08 11:30:56 EDT] submit_cluster_job() Launching job on cluster.
[2019-08-08 11:31:23 EDT] cluster_commands() Container image prepared: "CbrainTask::BIDSAppFreesurferReconAll.simg" (ID=2032289)
[2019-08-08 11:31:23 EDT] cluster_commands() Running bosh simulation invocation
[2019-08-08 11:31:27 EDT] cluster_commands() Simulation command failed: got status 99 and STDERR:
[2019-08-08 11:31:27 EDT] cluster_commands() [ ERROR ] u'cross-sectional' is not of type 'array'

Failed validating 'type' in schema['properties'][u'steps']:
[2019-08-08 11:31:27 EDT] setup_and_submit_job() Exception raised while setting up: CbrainError: Simulate command failed
prioux commented 5 years ago

Hum. Ok, I'll have a look...

prioux commented 5 years ago

Is it possible the descriptor is wrong? It says "list" in its type, are you supposed to be able to select multiple values? In the command-line would it be -steps cross-sectional template or what? If only one value is supposed to be selected, then the type is not supposed to be "list", just "string"....

shots47s commented 5 years ago

No, it is a list of values selected from a list of acceptable values, so it can be a list made up of elements of only those three choices.

prioux commented 5 years ago

I can make it so that the form properly generates a list, but there is no way I can make it support multiple values, those custom selection boxes can only be used to select one value... :-(

MontrealSergiy commented 5 years ago

I hear RoR can support multi select https://stackoverflow.com/questions/9280480/multiple-selection-in-ror-form-for

One of my first python task was adding Select 2 to a webapp. It was not as straightforward as I thought and took help of several people to addrss all the bugs and corner cases

prioux commented 5 years ago

Oh, we have lots of multiple selection inputs in CBRAIN, they're all supported fine by RoR; I'm talking about the task submission forms, which do not use standard elements, they built their own dropdowns and checkboxes.

shots47s commented 5 years ago

So we would need to build a new element? This whole thing screams of "rewrite me". Where are the elements created?

prioux commented 5 years ago

All the HTML templates and associated javascript code is in https://github.com/aces/cbrain/blob/master/BrainPortal/lib/cbrain_task_generators/templates/task_params.html.erb.erb

I don't think we want to rewrite it all, it's at least one man-year of work. It's good, solid and tested code (see the unit tests in BrainPortal/spec/boutiques/boutiques_tester_spec.rb and Bourreau/spec/boutiques/boutiques_tester_spec.rb).

prioux commented 5 years ago

My changes to make it a list was this:

diff --git a/BrainPortal/lib/cbrain_task_generators/templates/task_params.html.erb.erb b/BrainPortal/lib/cbrain_task_generators/templates/task_params.html.erb.erb
index fe903c1..bde260f 100644
--- a/BrainPortal/lib/cbrain_task_generators/templates/task_params.html.erb.erb
+++ b/BrainPortal/lib/cbrain_task_generators/templates/task_params.html.erb.erb
@@ -319,7 +319,8 @@
   <%- enum_vals = param["value-choices"] %>
   <%- if not enum_vals.nil? -%>
         # Enum value dropdown
-        dropdown.(id, id,
+        name = id <%= list ? '+ "[]"' : '' %>
+        dropdown.(id, name,
           <%= enum_vals.map { |v| [v, v] } %>,
           optional: <%= opt %>,
           default:  <%= "\"#{defaults.find { |d| d['id']==id }['default-value']}\"" rescue "nil" %>
prioux commented 5 years ago

I have searched through all the boutiques descriptors in the CBRAIN service, and I have not find any other instances of a multi list like in the original issue description. I supposed it's a really really rare way of definining an input.

Can the tool be invoked with distinct -steps arguments? E.g. -steps longitudinal -steps cross-sectional ? In that case I'd create three inputs, named steps1 steps2 and steps3 and allow the user to select one value from three drop downs... The advantage also is that it conserves order.

prioux commented 5 years ago

I looked at the code and I think it would not be too difficult to create a new dropdown for multi select lists. Basically it would look like the normal drop down but when selecting or deslecting an item, the params box would show the item list and a corresponding input tag (one per item) would be disabled or enabled.

I think this issue should be combined with adding support for list-separator in the boutiques descriptor (it could even be used in the form's params box).

prioux commented 5 years ago

I have made several implementation recommendations in a comment at the bottom of #861

prioux commented 5 years ago

@shots47s So do we keep this open until a multiselect dropdown widget has been coded? Have you looked at the code as described at bottom of #861 ?

shots47s commented 5 years ago

I haven't had a chance, but yes, let's keep it open. I will try to take a look by the end of the week.

natacha-beck commented 4 years ago

Fix by #911