From eea46dd8a425ce25ffbfe4c5780f01279fdc60ec Mon Sep 17 00:00:00 2001 From: guerler Date: Tue, 14 Jul 2026 17:13:56 +0300 Subject: [PATCH] Move color default to parse time, clarify deferred sniff and comments --- lib/galaxy/model/deferred.py | 7 ++++++- lib/galaxy/tool_util/parameters/convert.py | 12 ++++-------- lib/galaxy/tool_util/parameters/factory.py | 7 ++++++- lib/galaxy/tool_util_models/parameters.py | 4 ++-- test/unit/data/test_dataset_materialization.py | 10 +++++++--- 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/lib/galaxy/model/deferred.py b/lib/galaxy/model/deferred.py index cd9e2759c41..568c244210a 100644 --- a/lib/galaxy/model/deferred.py +++ b/lib/galaxy/model/deferred.py @@ -270,7 +270,12 @@ class DatasetInstanceMaterializer: path: str, dataset_instance: HistoryDatasetAssociation | LibraryDatasetDatasetAssociation, ) -> None: - """Replace ``extension="auto"`` with a sniffed extension after download.""" + """Resolve ``extension="auto"`` once the deferred source is streamed to ``path``. + + A deferred fetch/upload with no explicit ``ext`` is registered as ``auto`` because + the content is not available to sniff at request time. ``path`` is the first point + the bytes exist locally, so we sniff here while materializing. + """ if self._datatypes_registry is None: return if dataset_instance.extension != "auto": diff --git a/lib/galaxy/tool_util/parameters/convert.py b/lib/galaxy/tool_util/parameters/convert.py index c0c8eadf412..0cbcd88109c 100644 --- a/lib/galaxy/tool_util/parameters/convert.py +++ b/lib/galaxy/tool_util/parameters/convert.py @@ -492,14 +492,11 @@ def _fill_default_for(tool_state: dict[str, Any], parameter: ToolParameterT) -> # see test_tools.py -> expression_null_handling_boolean or test cases for gx_boolean_optional.xml tool_state[parameter_name] = parameter.value or False - if isinstance(parameter, (IntegerParameterModel, FloatParameterModel, HiddenParameterModel)): + if isinstance( + parameter, (IntegerParameterModel, FloatParameterModel, HiddenParameterModel, ColorParameterModel) + ): if parameter_name not in tool_state: tool_state[parameter_name] = parameter.value - elif isinstance(parameter, ColorParameterModel): - # Legacy ``value=""`` on an optional color means "no default"; treat as unset (arriba). - if parameter_name not in tool_state: - default = parameter.value - tool_state[parameter_name] = None if default == "" else default elif isinstance(parameter, GenomeBuildParameterModel): if parameter_name not in tool_state and parameter.optional: tool_state[parameter_name] = None @@ -636,8 +633,7 @@ def _initialize_repeat_state(parameter: RepeatParameterModel, tool_state: dict[s if parameter_name not in tool_state: tool_state[parameter_name] = [] repeat_instances = cast(list[dict[str, Any]], tool_state[parameter_name]) - # Match sync's ``Repeat.get_initial_value()``: seed ``max(min, default)`` empty - # instances when the request supplies none (bigscape). + # Seed ``max(min, default)`` empty instances when the request supplies none. floor = max(parameter.min or 0, parameter.default or 0) while len(repeat_instances) < floor: repeat_instances.append({}) diff --git a/lib/galaxy/tool_util/parameters/factory.py b/lib/galaxy/tool_util/parameters/factory.py index 712ca027bad..d20ed3f10c4 100644 --- a/lib/galaxy/tool_util/parameters/factory.py +++ b/lib/galaxy/tool_util/parameters/factory.py @@ -198,11 +198,16 @@ def _from_input_source_galaxy(input_source: InputSource, profile: float) -> Tool ) elif param_type == "color": optional = input_source.parse_optional() + color_value = get_color_value(input_source) + # A color default must be a valid color or None. The legacy ``value=""`` + # on an optional color means "unset". + if optional and color_value == "": + color_value = None return ColorParameterModel( type="color", name=input_source.parse_name(), optional=optional, - value=get_color_value(input_source), + value=color_value, **_common_param_kwargs(input_source), ) elif param_type == "rules": diff --git a/lib/galaxy/tool_util_models/parameters.py b/lib/galaxy/tool_util_models/parameters.py index 195cd3f8844..5c264ebd0e0 100644 --- a/lib/galaxy/tool_util_models/parameters.py +++ b/lib/galaxy/tool_util_models/parameters.py @@ -2397,8 +2397,8 @@ class RepeatParameterModel(BaseGalaxyToolParameterModelDefinition): parameters: list["ToolParameterT"] min: int | None = None max: int | None = None - # XML ````; matches sync's ``Repeat.get_initial_value()`` to seed - # N empty instances when the request supplies none. + # Legacy XML ````: number of empty instances to seed when the + # request supplies none. default: int = 0 def field_kwargs(self) -> dict[str, Any]: diff --git a/test/unit/data/test_dataset_materialization.py b/test/unit/data/test_dataset_materialization.py index 620362b854e..924f19366f0 100644 --- a/test/unit/data/test_dataset_materialization.py +++ b/test/unit/data/test_dataset_materialization.py @@ -435,8 +435,11 @@ def test_deferred_hdas_with_deferred_metadata(): def test_deferred_hda_with_auto_extension_gets_sniffed_from_content(): - # An HDA created with extension="auto" should get a real extension after materialize - # when a datatypes_registry is provided. + # Scenario: a deferred fetch/upload with no explicit ``ext`` is stored as + # extension="auto" because the bytes aren't available to sniff at request time + # (data_fetch.py: ``requested_ext = item.get("ext", "auto")``). When the dataset is + # later materialized for a tool run, the real type is sniffed from the downloaded + # content. Requires a datatypes_registry. fixture_context = setup_fixture_context_with_history() store_dict = deferred_hda_model_store_dict() store_dict["datasets"][0]["extension"] = "auto" @@ -460,7 +463,8 @@ def test_deferred_hda_with_auto_extension_gets_sniffed_from_content(): def test_deferred_hda_without_registry_keeps_auto_extension(): - # Without a datatypes_registry the sniff step is a no-op; extension stays "auto". + # Guards backward compatibility: existing materializer callers that don't pass a + # datatypes_registry keep the prior behavior (no sniffing); extension stays "auto". fixture_context = setup_fixture_context_with_history() store_dict = deferred_hda_model_store_dict() store_dict["datasets"][0]["extension"] = "auto"