Use visit inputs for check_and_update_param_values#1764
Use visit inputs for check_and_update_param_values#1764jmchilton merged 4 commits intogalaxyproject:devfrom
Conversation
ac95c7b to
11ccedc
Compare
2a03e37 to
45930b7
Compare
45930b7 to
4a956a9
Compare
f9186f6 to
a4b8728
Compare
a4b8728 to
02187af
Compare
| return | ||
| for validator in self.validators: | ||
| validator.validate( value, trans ) | ||
| if value is not '' or not self.optional: |
There was a problem hiding this comment.
if not value or not self.optional?
There was a problem hiding this comment.
Its a revision of the above line, saying that values which are empty and optional are not validated, but optional values which are set to a non-empty value are validated. For more consistency it would be good to change this to if value is not None or not self.optional: in the future.
| # their state in workflow encoding. | ||
| if input.is_job_resource_conditional: | ||
| if input.name == '__job_resource': | ||
| if input.name in values: |
There was a problem hiding this comment.
I'm going to merge this - but I don't understand your problem with giving names to repeated computations. You are duplicating the check and the resulting if check is both more obtuse and implementation dependent. As a result, this change makes things less readable and more easy to introduce bugs into.
There was a problem hiding this comment.
Ok, so I moved this customization out of the common conditional element code since it is only used for job resources and only consists of the above line. I think it is easier to read too (: but that is obviously subjective, so I am open for suggestions. Thanks a lot for your help. I really appreciate it.
Use visit inputs for check_and_update_param_values
Replaces remaining custom iterations (https://github.com/galaxyproject/galaxy/pull/1764/files#diff-1bef786a2b2ea704c1748e41cd25dd33L1283 and https://github.com/galaxyproject/galaxy/pull/1764/files#diff-ca2e00a05d3404fc5410df390ed69ac1L116) with the common visit inputs caller (guerler@e732f5c#diff-c2872901d34ca0c09ded74872356dd7bL13). Also removes the dependency on
__current_case__for conditionals. Thanks @jmchilton. Additional the PR moves the job resource handling to the job configuration module.