Conditional outputs#303 #549

DarrinFong · 2019-10-16T20:46:57Z

Purpose

Enables Conditional-path-template in descriptor's output-files. CPT will contain a list of objects whose key represents a boolean expression (python syntax), the first true expression's corresponding path will be selected (top down). If the output-file is required, a default key is must be included in the list of CPT.

PT and CPT are mutex properties within an output-file

Current behaviour

"output-files": [{
  ... ,
  "path-template": "[PARAM1].txt"
}]

New behaviour

"output-files": [{
  ... ,
  "path-template": "[PARAM1].txt"
}, {
  ... , 
  "optional": false,
  "conditional-path-template":
  [
    {"opt1 and (opt2 > 10)": "[OPT1]-thing_large.txt"},
    {"opt1":                 "[OPT1]-thing.txt"},
    {"opt2 > 10":            "[PARAM1]-thing_large.txt"},
    {"default":              "[PARAM1]-thing.txt"}
  ]
}]

…ing file name generation

…le1tool to generate output files and validate generation boutiques#303

glatard · 2019-10-17T13:56:58Z

Hi @DarrinFong, tests are currently failing, would you like to have a look?

coveralls · 2019-10-17T20:47:14Z

Coverage increased (+2.08%) to 93.957% when pulling 3cdc5ab on DarrinFong:conditional_outputs#303 into 0469a3d on boutiques:develop.

coveralls · 2019-10-17T20:47:14Z

Coverage decreased (-0.1%) to 93.905% when pulling e566b3e on DarrinFong:conditional_outputs#303 into f904c2a on boutiques:develop.

tools/python/boutiques/validator.py

Release 0.5.22.post2

tools/python/boutiques/localExec.py

gkiar · 2019-11-05T18:08:15Z

tools/python/boutiques/localExec.py

+                        parsedExp.append(word)
+                # If expression is true, set fileName
+                # Stop checking (if-elif...)
+                if " ".join("{0}".format(w) for w in parsedExp) == "default":


I may be reading this wrong, but aren't what you have written on #L1078 (parsedExp = ["False"]) and what you're checking here, ( .... == "default") incompatible, so defaults would never be added? You know what's going on in here much better than I, @DarrinFong, so please just double check and let me know :)

if word in in_out_dict: # in_out_dict are k:v pairs that will be in the output of simulate elif word in all_ids: parsedExp = ["False"] break

The elif block should only 'falsify' the expressions that contain an ID which is both:

in descriptor's inputs/output-files IDs

NOT in selected values for simulate

Which means the only way for 'default' to be evaluated False is when an ID of name 'default' is present in the descriptor's inputs/output-files IDs. (This raises a concern of whether we have to add 'default' to the list of invalid IDs).

Great - thank you for the clarification :)

I do think that maybe we want to add "default" as a restricted word in the schema so that it can't be used as an ID... @glatard, thoughts?

gkiar · 2019-11-05T18:09:10Z

Hey @DarrinFong - I added some comments. Overall, this looks awesome! Thanks so much :)

…ession variables exist in descriptor inputs/output-files). Added unit tests for those.

…perties (review), attempting check variable against type of input for CPT's expressions

gkiar · 2019-11-11T18:50:53Z

@DarrinFong - want to investigate why this is failing, and we can re-review once that is fixed? Thanks!

…arator

…or: invalid keywords, cross-type comparison. Unit test created

…ents checkfor and/or. Updated schema description for conditional path template

…nal_outputs#303 Merging commit into branch to fix broken travis, caused by defining python specific dependencies

…ng/boutiques into conditional_outputs#303

glatard · 2019-12-12T20:12:43Z

tools/python/requirements.txt

+docopt==0.6.2
+nexus-sdk; 
+python_version >= '3.5';


This will break installation from pypi for Python<3.5. The solution has to work with the following install command:

pip install -r requirements.txt

For Python<3.5.
The nexus line should be written as:
nexus-sdk; python_version >= '3.5'

DarrinFong · 2019-12-13T05:32:00Z

@gkiar Hey Greg, can you take a look at the conditions validation you mentioned in a conversation above?
The current validation is to recursively (by parentheses) replace the values neighbouring a valid operator (==, !=, >=, <=, >, <) by their types (Flag, Number, String, Path) and check if they are of the same type. Thanks a lot!

gkiar · 2019-12-16T15:26:46Z

tools/python/boutiques/validator.py

@@ -58,6 +62,82 @@ def inById(i):
            return descriptor["inputs"][inputGet("id").index(i)]
        return {}

+    def conditionalExpFormat(s):


This function appears to exist in both localExec.py and validator.py, so perhaps we could move it to a utils file and import it?

Function moved to utils.py!

tools/python/boutiques/validator.py

gkiar · 2019-12-16T15:32:32Z

tools/python/boutiques/validator.py

@@ -22,6 +23,9 @@ def validate_descriptor(json_file, **kwargs):
    """
    path, fil = op.split(bfile)
    schema_file = op.join(path, "schema", "descriptor.schema.json")
+    allowed_keywords = ['and', 'or', 'false', 'true']


Because capitalization may be funny/distinct across representations (in particular True versus true), we may want to either a) add more capitalizations to this list, or b) convert everything to an upper/lowercase value when we compare. Essentially just an extra precaution to avoid the hole of evaluating a JSON true and a boolean Python True to be distinct.

I opted for option b), just in case someone needs to write ANd 😄

gkiar

Hey @DarrinFong - thanks for this! It looks awesome, I'm really excited to have this rolled in 😄
I left a few comments, but they're all minor and non-blocking if you're too swamped to get to them/we can just create issues to tie up the ends. Lmk if what you think!

Thanks again :D

… Moved redundant condiitonalExpFormat into utils.py

…ions

DarrinFong and others added 9 commits October 2, 2019 14:21

Added test case and test data for conditional output names, implement…

0c5e9f8

…ing file name generation

more test cases

72eea28

bexample for fixed and conditonal output file names works with -c

f8026f5

bexample for conditional output file names finished

8ef9fc8

Finished tests for conditional output file names, added code in examp…

556d8a4

…le1tool to generate output files and validate generation boutiques#303

Added changes for python backwards compatibility

df6674c

Merge branch 'develop' into conditional_outputs#303

3b81070

output-files is a non required list

9aea14c

syntax change for test assertion

83ee756

DarrinFong added 4 commits October 17, 2019 13:36

removed non compatible code, tests pass locally

348cd1c

codestyle change

3c22dcf

re-tested for p2.7 and 3.6

f63526a

error message discrepancy between local & travis

e566b3e

DarrinFong added 2 commits October 17, 2019 16:51

unsorted launch output caused error

99954b2

unsorted launch output caused error

26e7d96

gkiar reviewed Oct 28, 2019

View reviewed changes

tools/python/boutiques/validator.py Show resolved Hide resolved

Merge pull request boutiques#558 from boutiques/develop

94f7dd1

Release 0.5.22.post2

gkiar reviewed Nov 5, 2019

View reviewed changes

tools/python/boutiques/localExec.py Outdated Show resolved Hide resolved

gkiar reviewed Nov 5, 2019

View reviewed changes

tools/python/boutiques/localExec.py Outdated Show resolved Hide resolved

gkiar reviewed Nov 5, 2019

View reviewed changes

DarrinFong added 4 commits November 6, 2019 11:21

refactored outFileName generation for path-template/CPT and commented

7738970

Added validator for input validity (unique default condition and expr…

ad919a9

…ession variables exist in descriptor inputs/output-files). Added unit tests for those.

Modified descriptor schema for id and conditional-path-template's pro…

5d0f974

…perties (review), attempting check variable against type of input for CPT's expressions

removed test files

69bb734

type checking input variables against value on the other side of comp…

3428be9

…arator

DarrinFong added 8 commits November 26, 2019 18:38

Conditional path template's expressions are validated. Error raised f…

57f891f

…or: invalid keywords, cross-type comparison. Unit test created

bugfix

a55f6d3

removed duplicate output file test for example1, added neighbour elem…

dc0bd33

…ents checkfor and/or. Updated schema description for conditional path template

restored setup, updated requirements.txt

72110a4

Merge commit '94f7dd14485666752b1e86fc8c20c54663ec6cb0' into conditio…

67c1835

…nal_outputs#303 Merging commit into branch to fix broken travis, caused by defining python specific dependencies

restoring d3cf873 and added requirements

7ea6cf6

restoring d3cf873 and added requirements

6e2f5d7

Merge branch 'conditional_outputs#303' of https://github.com/DarrinFo…

1f093b9

…ng/boutiques into conditional_outputs#303

glatard reviewed Dec 12, 2019

View reviewed changes

Fixed requirement.txt

d3cdd41

gkiar reviewed Dec 16, 2019

View reviewed changes

tools/python/boutiques/validator.py Show resolved Hide resolved

gkiar reviewed Dec 16, 2019

View reviewed changes

DarrinFong added 2 commits December 16, 2019 18:04

Added integer type checking in conditional path template expressions.…

ea90f62

… Moved redundant condiitonalExpFormat into utils.py

removed Number type validation of integer for condtional path express…

3cdc5ab

…ions

gkiar merged commit b79df4e into boutiques:develop Dec 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Conditional outputs#303 #549

Conditional outputs#303 #549

DarrinFong commented Oct 16, 2019

glatard commented Oct 17, 2019

coveralls commented Oct 17, 2019 •

edited

Loading

coveralls commented Oct 17, 2019

gkiar Nov 5, 2019

DarrinFong Nov 6, 2019 •

edited

Loading

gkiar Nov 6, 2019

gkiar commented Nov 5, 2019

gkiar commented Nov 11, 2019

glatard Dec 12, 2019

DarrinFong commented Dec 13, 2019 •

edited

Loading

gkiar Dec 16, 2019

DarrinFong Dec 16, 2019

gkiar Dec 16, 2019

DarrinFong Dec 16, 2019

gkiar left a comment

Conditional outputs#303 #549

Conditional outputs#303 #549

Conversation

DarrinFong commented Oct 16, 2019

Purpose

Current behaviour

New behaviour

glatard commented Oct 17, 2019

coveralls commented Oct 17, 2019 • edited Loading

coveralls commented Oct 17, 2019

gkiar Nov 5, 2019

Choose a reason for hiding this comment

DarrinFong Nov 6, 2019 • edited Loading

Choose a reason for hiding this comment

gkiar Nov 6, 2019

Choose a reason for hiding this comment

gkiar commented Nov 5, 2019

gkiar commented Nov 11, 2019

glatard Dec 12, 2019

Choose a reason for hiding this comment

DarrinFong commented Dec 13, 2019 • edited Loading

gkiar Dec 16, 2019

Choose a reason for hiding this comment

DarrinFong Dec 16, 2019

Choose a reason for hiding this comment

gkiar Dec 16, 2019

Choose a reason for hiding this comment

DarrinFong Dec 16, 2019

Choose a reason for hiding this comment

gkiar left a comment

Choose a reason for hiding this comment

coveralls commented Oct 17, 2019 •

edited

Loading

DarrinFong Nov 6, 2019 •

edited

Loading

DarrinFong commented Dec 13, 2019 •

edited

Loading