boutiques / boutiques

A cross-platform descriptive command-line framework for applications
http://boutiques.github.io
Other
59 stars 49 forks source link

exception when one value of a param both disables and requires some inputs while another value does not #649

Open MontrealSergiy opened 2 years ago

MontrealSergiy commented 2 years ago

the problem is actually double-fold (a) the restriction of value-requires/disables being for each allowed value are unnecessary strict, and (b) when this overly narrow restrictions is violated, instead of proper error message bosh yields a python exec trace and exception (keyerror)

The latter problem is most severe yet it would be great to relax unnecessary strict restriction too.

the code is wrongfully assumes that if one of values of an input disables and requires some input(s) then all the values of that parameter require and disable something. TIf this assumption does not hold, a keyerror exception might occur. For example

        {
            "name":              "a dropdown, causing exception in bosh validate",
            "id":                "dropdown",
            "description":       "x or y",
            "type":              "String",
            "value-choices":     ["x", "y"],
            "value-requires":
                {"x":  ["du_report_out" ]},
            "value-disables":                {"x":  ["option_a" ]},
            "requires-inputs": ["option_upper_h"],
            "disables-inputs": ["option_h", "minput2"],
            "value-key":         "[DROP]"
        }

Expected Behavior

bosh validation runs without exception (resulting in ok or user friendly error message, if you still believe that for some weird reason that all value-choices should be represented in value-requires or value-disables, )

Current Behavior

for the input example see the beginning of the bug report, exception

bosh validate boutiques.139-1.json
Traceback (most recent call last):
  File "/home/users/sboroday/bin/bosh", line 11, in <module>
    sys.exit(bosh())
  File "/home/users/sboroday/.local/lib/python3.6/site-packages/boutiques/bosh.py", line 962, in bosh
    out = validate(*params)
  File "/home/users/sboroday/.local/lib/python3.6/site-packages/boutiques/bosh.py", line 103, in validate
    sandbox=results.sandbox)
  File "/home/users/sboroday/.local/lib/python3.6/site-packages/boutiques/validator.py", line 368, in validate_descriptor
    for choice in inp["value-choices"]
  File "/home/users/sboroday/.local/lib/python3.6/site-packages/boutiques/validator.py", line 369, in <listcomp>
    for ids1 in inp["value-disables"][choice]
KeyError: 

Environment (optional)

Recent version, Python 3.5.2

Context (optional)

it does not affect me (so far)

Possible fix (optional)

The exception occurs because comparison of value-require/disable keys with value choices occurs after a loop over all values (one that check that no input is simultaneously disabled and enable at line 364) . So to avoid exception is enough to swap these checks.

To address both a) and b),

unless there is other dependent code, drop

if set(inp[param].keys()) != set(inp["value-choices"])

and

diff --git a/boutiques/validator.py b/boutiques/validator.py
index 9c35016..0a0f6b0 100755
--- a/boutiques/validator.py
+++ b/boutiques/validator.py
@@ -363,7 +363,7 @@ def validate_descriptor(descriptor, **kwargs):
                                 " and disables \"{2}\"")
                 errors += [msg_template.format(inp["id"], choice, ids1)
                            for choice in inp["value-choices"]
-                           for ids1 in inp["value-disables"][choice]
+                           for ids1 in inp["value-disables"].get(choice, [])
-                           if ids1 in inp["value-requires"][choice]]
+                           if ids1 in inp["value-requires"].get(choice, [])]

             for param in ["value-requires", "value-disables"]:

but could be also

                choices = set(inp["value_disables"]) & set(inp["value-requires"])
                errors += [msg_template.format(inp["id"], choice, ids1)
                           for choice in choices

also comment could benefit from some rephrasing

-            # Verify not value not requiring and disabling input
+            # Verify that no value disables and requires a very same input
            `

Related issues (optional)

MontrealSergiy commented 2 years ago

a complete descriptor for you https://github.com/MontrealSergiy/cbrain-plugins-test/blob/aea472bdf36eb7be35d78cb41cebe7f324265fb6/boutiques_descriptors/dependencyhell.json