Skip to content

fix: serialize typed values in ChoiceListParameter - #3440

Open
winklemad wants to merge 1 commit into
spotify:masterfrom
winklemad:fix/choicelist-serialize-typed
Open

fix: serialize typed values in ChoiceListParameter#3440
winklemad wants to merge 1 commit into
spotify:masterfrom
winklemad:fix/choicelist-serialize-typed

Conversation

@winklemad

Copy link
Copy Markdown

Description

ChoiceListParameter.serialize joins the raw values:

def serialize(self, x):
    return self._sep.join(x)

But str.join requires every element to be a str, while ChoiceListParameter.parse produces values of self._var_type (return self.normalize(map(self._var_type, values))). So with var_type=int (or float), serialize raises TypeError, and the parse/serialize round trip is broken by construction.

Because serialize is called from Task.to_str_params during task_id computation, an int/float ChoiceListParameter is completely unusable — instantiating any task that uses one throws:

class Foo(luigi.Task):
    args = luigi.ChoiceListParameter(choices=[1, 2, 3], var_type=int)

Foo(args=(1, 3))
# TypeError: sequence item 0: expected str instance, int found

int choices are a documented, typed use case (test/mypy_test.py), and the sibling ChoiceParameter (via the default str(x) serializer) and EnumListParameter (self._sep.join([e.name for e in x])) both handle non-str values correctly — only ChoiceListParameter joins raw typed values.

Fix

Serialize each value with str(), matching EnumListParameter:

return self._sep.join(str(v) for v in x)

No change for the existing var_type=str case (str("1") == "1"); it restores the parse/serialize round trip for int/float.

Testing

Added test_choice_list_param_typed_serialize_parse, asserting int and float ChoiceListParameter serialize and round-trip correctly, with the str case unchanged. It fails before the change (TypeError) and passes after; test/parameter_test.py passes and ruff check / ruff format --check are clean.

ChoiceListParameter.serialize did self._sep.join(x), which raises TypeError
for any non-str var_type (int/float) because str.join requires str elements.
parse() produces var_type values via map(self._var_type, ...), so the
parse/serialize round trip was broken and an int/float ChoiceListParameter
was unusable -- instantiating a task with one throws during task_id
computation. Serialize each value with str(), matching EnumListParameter.
@winklemad
winklemad requested a review from dlstadther as a code owner July 11, 2026 18:27
@winklemad
winklemad requested a review from a team July 11, 2026 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant