Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[24.2] Fix default value handling for parameters connected to required parameters #19219

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions lib/galaxy/model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8174,10 +8174,6 @@ def input_type(self):
assert self.is_input_type, "step.input_type can only be called on input step types"
return self.STEP_TYPE_TO_INPUT_TYPE[self.type]

@property
def input_default_value(self):
self.get_input_default_value(None)

def get_input_default_value(self, default_default):
# parameter_input and the data parameters handle this slightly differently
# unfortunately.
Expand Down
2 changes: 2 additions & 0 deletions lib/galaxy/tool_util/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ class TestJob(StrictModel):
doc: Optional[str]
job: JobDict
outputs: Dict[str, TestOutputAssertions]
expect_failure: Optional[bool] = False


Tests = RootModel[List[TestJob]]
Expand All @@ -185,6 +186,7 @@ class TestJob(StrictModel):
class TestJobDict(TypedDict):
doc: NotRequired[str]
job: NotRequired[JobDict]
expect_failure: NotRequired[bool]
outputs: OutputsDict


Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/tool_util/parser/parameter_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ class EmptyFieldParameterValidatorModel(StaticValidatorModel):

@staticmethod
def empty_validate(value: Any, validator: "ValidatorDescription"):
raise_error_if_valiation_fails((value != ""), validator)
raise_error_if_valiation_fails((value not in ("", None)), validator)

def statically_validate(self, value: Any) -> None:
EmptyFieldParameterValidatorModel.empty_validate(value, self)
Expand Down
6 changes: 1 addition & 5 deletions lib/galaxy/tools/parameters/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -368,11 +368,7 @@ def __init__(self, tool, input_source):

def to_json(self, value, app, use_security):
"""Convert a value to a string representation suitable for persisting"""
if value is None:
rval = "" if not self.optional else None
else:
rval = unicodify(value)
return rval
return unicodify(value)

def get_initial_value(self, trans, other_values):
return self.value
Expand Down
13 changes: 8 additions & 5 deletions lib/galaxy/workflow/modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -1260,12 +1260,15 @@ def get_inputs(self):

when_true = ConditionalWhen()
when_true.value = "true"
when_true.inputs = {}
when_true.inputs["default"] = specify_default_cond
when_true.inputs = {"default": specify_default_cond}

when_false = ConditionalWhen()
when_false.value = "false"
when_false.inputs = {}
# This is only present for backwards compatibility,
# We don't need this conditional since you can set
# a default value for optional and required parameters.
# TODO: version the state and upgrade it to a simpler version
when_false.inputs = {"default": specify_default_cond}

optional_cases = [when_true, when_false]
optional_cond.cases = optional_cases
Expand Down Expand Up @@ -1504,6 +1507,7 @@ def get_runtime_inputs(self, step, connections: Optional[Iterable[WorkflowStepCo
parameter_def = self._parse_state_into_dict()
parameter_type = parameter_def["parameter_type"]
optional = parameter_def["optional"]
default_value_set = "default" in parameter_def
default_value = parameter_def.get("default", self.default_default_value)
if parameter_type not in ["text", "boolean", "integer", "float", "color", "directory_uri"]:
raise ValueError("Invalid parameter type for workflow parameters encountered.")
Expand Down Expand Up @@ -1557,7 +1561,7 @@ def _parameter_def_list_to_options(parameter_value):

parameter_class = parameter_types[client_parameter_type]

if optional:
if default_value_set:
if client_parameter_type == "select":
parameter_kwds["selected"] = default_value
else:
Expand Down Expand Up @@ -1633,7 +1637,6 @@ def step_state_to_tool_state(self, state):
if "default" in state:
default_set = True
default_value = state["default"]
state["optional"] = True
multiple = state.get("multiple")
source_validators = state.get("validators")
restrictions = state.get("restrictions")
Expand Down
7 changes: 4 additions & 3 deletions lib/galaxy/workflow/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,8 +574,8 @@ def set_outputs_for_input(
if self.inputs_by_step_id:
step_id = step.id
if step_id not in self.inputs_by_step_id and "output" not in outputs:
default_value = step.input_default_value
if default_value or step.input_optional:
default_value = step.get_input_default_value(modules.NO_REPLACEMENT)
if default_value is not modules.NO_REPLACEMENT:
outputs["output"] = default_value
else:
log.error(f"{step.log_str()} not found in inputs_step_id {self.inputs_by_step_id}")
Expand All @@ -588,7 +588,8 @@ def set_outputs_for_input(
)
)
elif step_id in self.inputs_by_step_id:
outputs["output"] = self.inputs_by_step_id[step_id]
if self.inputs_by_step_id[step_id] is not None or "output" not in outputs:
outputs["output"] = self.inputs_by_step_id[step_id]

if step.label and step.type == "parameter_input" and "output" in outputs:
self.runtime_replacements[step.label] = str(outputs["output"])
Expand Down
4 changes: 3 additions & 1 deletion lib/galaxy_test/api/test_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -3212,7 +3212,7 @@ def test_export_invocation_ro_crate_adv(self):
""",
test_data="""
num_lines_param:
type: int
type: raw
value: 2
input collection 1:
collection_type: list
Expand Down Expand Up @@ -7034,12 +7034,14 @@ def test_subworkflow_import_order_maintained(self, history_id):
outer_input_1:
type: int
default: 1
optional: true
position:
left: 0
top: 0
outer_input_2:
type: int
default: 2
optional: true
position:
left: 100
top: 0
Expand Down
2 changes: 0 additions & 2 deletions lib/galaxy_test/base/populators.py
Original file line number Diff line number Diff line change
Expand Up @@ -3362,8 +3362,6 @@ def read_test_data(test_dict):
hda = dataset_populator.new_dataset(history_id, content=value)
label_map[key] = dataset_populator.ds_entry(hda)
inputs[key] = hda
else:
raise ValueError(f"Invalid test_data def {test_data}")

return inputs, label_map, has_uploads

Expand Down
1 change: 1 addition & 0 deletions lib/galaxy_test/base/workflow_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,7 @@
int_input:
type: integer
default: 3
optional: true
steps:
random:
tool_id: random_lines1
Expand Down
34 changes: 34 additions & 0 deletions lib/galaxy_test/workflow/default_values.gxwf-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
- doc: |
Test that default value doesn't need to be supplied
job: {}
outputs:
out:
class: File
asserts:
- that: has_text
text: "1"
- doc: |
Test that null is replaced with default value (follows https://www.commonwl.org/v1.2/Workflow.html#WorkflowInputParameter)
job:
required_int_with_default:
type: raw
value: null
outputs:
out:
class: File
asserts:
- that: has_text
text: "1"
- doc: |
Test that empty string is not replaced and fails
expect_failure: true
job:
required_int_with_default:
type: raw
value: ""
outputs:
out:
class: File
asserts:
- that: has_text
text: "1"
17 changes: 17 additions & 0 deletions lib/galaxy_test/workflow/default_values.gxwf.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
class: GalaxyWorkflow
inputs:
required_int_with_default:
type: int
default: 1
outputs:
out:
outputSource: integer_default/out_file1
steps:
integer_default:
tool_id: integer_default
tool_state:
input1: 0
input2: 0
in:
input3:
source: required_int_with_default
34 changes: 34 additions & 0 deletions lib/galaxy_test/workflow/default_values_optional.gxwf-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
- doc: |
Test that default value doesn't need to be supplied
job: {}
outputs:
out:
class: File
asserts:
- that: has_text
text: "1"
- doc: |
Test that null is replaced with default value (follows https://www.commonwl.org/v1.2/Workflow.html#WorkflowInputParameter)
job:
optional_int_with_default:
type: raw
value: null
outputs:
out:
class: File
asserts:
- that: has_text
text: "1"
- doc: |
Test that empty string is not replaced and fails
expect_failure: true
job:
optional_int_with_default:
type: raw
value: ""
outputs:
out:
class: File
asserts:
- that: has_text
text: "1"
18 changes: 18 additions & 0 deletions lib/galaxy_test/workflow/default_values_optional.gxwf.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
class: GalaxyWorkflow
inputs:
optional_int_with_default:
type: int
default: 1
optional: true
outputs:
out:
outputSource: integer_default/out_file1
steps:
integer_default:
tool_id: integer_default
tool_state:
input1: 0
input2: 0
in:
input3:
source: optional_int_with_default
25 changes: 17 additions & 8 deletions lib/galaxy_test/workflow/test_framework_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,23 @@ def test_workflow(self, workflow_path: Path, test_job: TestJobDict):
with workflow_path.open() as f:
yaml_content = ordered_load(f)
with self.dataset_populator.test_history() as history_id:
run_summary = self.workflow_populator.run_workflow(
yaml_content,
test_data=test_job["job"],
history_id=history_id,
)
if TEST_WORKFLOW_AFTER_RERUN:
run_summary = self.workflow_populator.rerun(run_summary)
self._verify(run_summary, test_job["outputs"])
exc = None
try:
run_summary = self.workflow_populator.run_workflow(
yaml_content,
test_data=test_job["job"],
history_id=history_id,
)
if TEST_WORKFLOW_AFTER_RERUN:
run_summary = self.workflow_populator.rerun(run_summary)
self._verify(run_summary, test_job["outputs"])
except Exception as e:
exc = e
if test_job.get("expect_failure"):
if not exc:
raise Exception("Expected workflow test to fail but it passed")
elif exc:
raise exc

def _verify(self, run_summary: RunJobsSummary, output_definitions: OutputsDict):
for output_name, output_definition in output_definitions.items():
Expand Down
Loading