Skip to content

Commit

Permalink
Deserialize null to tool parameter default value
Browse files Browse the repository at this point in the history
I'm not 100% certain about this commit, but it does make some sense:

We might be setting optional step outputs to null if they're not
provided or if they are provided as null. We do have tests that require
that tool default values are used when an optional parameter is not
provided, so it seems like this is a necessity.

The advantage is that this is pretty easy to reason about, the downside
is that you can't provide a null value to an optional parameter that
specifies a non-null default value. This might seem nonsensical at first
since you might just drop the default value, but if a tool can accept
null but the default should be not-null, you still might want to
override the default with a null (in a workflow or tool run).

An alternative fix would need to distinguish unset and null, and would
need to persist the unset value in the database, or we'd have to rework
the scheduling code so that unset step outputs don't cause scheduling
errors.

Maybe this is good enough for now ?
  • Loading branch information
mvdbeek committed Dec 17, 2024
1 parent 8262dad commit e94d2f7
Showing 1 changed file with 12 additions and 0 deletions.
12 changes: 12 additions & 0 deletions lib/galaxy/tools/parameters/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ def to_json(self, value, app, use_security) -> str:

def to_python(self, value, app):
"""Convert a value created with to_json back to an object representation"""
if value is None and (default_value := getattr(self, "value", None)) not in ("", None):
# None should be equivalent to no value provided, so use default
return default_value
return value

def value_to_basic(self, value, app, use_security=False):
Expand Down Expand Up @@ -494,6 +497,9 @@ def to_python(self, value, app):
except (TypeError, ValueError) as err:
if contains_workflow_parameter(value):
return value
if value is None and self.value not in ("", None):
# None should be equivalent to no value provided, so use default
return int(self.value)
if not value and self.optional:
return None
raise err
Expand Down Expand Up @@ -567,6 +573,9 @@ def to_python(self, value, app):
except (TypeError, ValueError) as err:
if contains_workflow_parameter(value):
return value
if value is None and self.value not in ("", None):
# None should be equivalent to no value provided, so use default
return float(self.value)
if not value and self.optional:
return None
raise err
Expand Down Expand Up @@ -623,6 +632,9 @@ def from_json(self, value, trans, other_values=None):
return self.to_python(value)

def to_python(self, value, app=None):
if value is None and self.checked is not None:
# None should be equivalent to no value provided, so use default
return self.checked
if not self.optional:
ret_val = string_as_bool(value)
else:
Expand Down

0 comments on commit e94d2f7

Please sign in to comment.