Skip to content

Commit e544aef

Browse files
committed
fix: handle list-valued options in PROPERTY_MAPPING matching (#228)
Convert list values to tuples in _process_found_property_value() before wrapping in frozenset, preventing TypeError on unhashable list types while preserving element order. Closes #228
1 parent f1dce7e commit e544aef

File tree

2 files changed

+58
-0
lines changed

2 files changed

+58
-0
lines changed

mloda/core/abstract_plugins/components/feature_chainer/feature_chain_parser.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,8 @@ def _extract_property_values(cls, property_value: Any) -> Any:
198198
def _process_found_property_value(
199199
cls, found_property_value: Any, property_value: Any, property_name: str, original_property_config: Any
200200
) -> Set[str]:
201+
if isinstance(found_property_value, list):
202+
found_property_value = tuple(found_property_value)
201203
if not isinstance(found_property_value, frozenset):
202204
found_property_value = frozenset([found_property_value])
203205

tests/test_core/test_abstract_plugins/test_components/feature_chainer/test_feature_chain_parser_mixin.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,62 @@ def test_input_features_preserves_feature_order(self) -> None:
345345
assert feature_names == {"first", "second", "third"}
346346

347347

348+
class TestFeatureChainParserMixinListValuedOptions:
349+
"""Tests for list-valued options in PROPERTY_MAPPING (issue #228)."""
350+
351+
def test_match_feature_group_criteria_list_valued_option(self) -> None:
352+
"""Test that match_feature_group_criteria works with list-valued options."""
353+
354+
class ListValuedFeatureGroup(FeatureChainParserMixin):
355+
PROPERTY_MAPPING = {
356+
"partition_by": {
357+
"explanation": "List of columns to partition by",
358+
DefaultOptionKeys.context: True,
359+
DefaultOptionKeys.strict_validation: False,
360+
},
361+
}
362+
363+
options = Options(context={"partition_by": ["region", "category"]})
364+
result = ListValuedFeatureGroup.match_feature_group_criteria("my_feature", options)
365+
assert result is True
366+
367+
def test_match_feature_group_criteria_tuple_valued_option(self) -> None:
368+
"""Test that match_feature_group_criteria works with tuple-valued options."""
369+
370+
class TupleValuedFeatureGroup(FeatureChainParserMixin):
371+
PROPERTY_MAPPING = {
372+
"partition_by": {
373+
"explanation": "Columns to partition by",
374+
DefaultOptionKeys.context: True,
375+
DefaultOptionKeys.strict_validation: False,
376+
},
377+
}
378+
379+
options = Options(context={"partition_by": ("region", "category")})
380+
result = TupleValuedFeatureGroup.match_feature_group_criteria("my_feature", options)
381+
assert result is True
382+
383+
def test_list_valued_option_preserves_order(self) -> None:
384+
"""Test that list-valued options preserve element order through tuple conversion."""
385+
from mloda.core.abstract_plugins.components.feature_chainer.feature_chain_parser import FeatureChainParser
386+
387+
property_mapping = {
388+
"partition_by": {
389+
DefaultOptionKeys.context: True,
390+
DefaultOptionKeys.strict_validation: False,
391+
},
392+
}
393+
options = Options(context={"partition_by": ["col1", "col2", "col3"]})
394+
result = FeatureChainParser._validate_options_against_property_mapping(options, property_mapping)
395+
assert result is True
396+
397+
def test_scalar_option_still_works(self) -> None:
398+
"""Test that scalar (non-list) options still work after the fix."""
399+
options = Options(context={"operation": "op1"})
400+
result = MockFeatureGroup.match_feature_group_criteria("my_feature", options)
401+
assert result is True
402+
403+
348404
class TestFeatureChainParserMixinExtractSourceFeatures:
349405
"""Tests for _extract_source_features() classmethod."""
350406

0 commit comments

Comments
 (0)