Skip to content

fix: handle list-valued options in PROPERTY_MAPPING matching#230

Merged
TomKaltofen merged 2 commits intomainfrom
fix/property-mapping-list-valued-options
Mar 24, 2026
Merged

fix: handle list-valued options in PROPERTY_MAPPING matching#230
TomKaltofen merged 2 commits intomainfrom
fix/property-mapping-list-valued-options

Conversation

@TKaltofen
Copy link
Copy Markdown
Collaborator

Summary

  • Fixes _process_found_property_value() to convert list values to tuples before wrapping in frozenset, preventing TypeError: unhashable type: 'list' when using list-valued options in PROPERTY_MAPPING
  • Preserves element order (tuples are ordered, unlike frozensets)
  • Adds 4 tests covering list-valued, tuple-valued, order preservation, and scalar regression

Test plan

  • New tests pass: test_match_feature_group_criteria_list_valued_option, test_match_feature_group_criteria_tuple_valued_option, test_list_valued_option_preserves_order, test_scalar_option_still_works
  • All 29 feature chain parser tests pass
  • Full tox run: 2178 passed (16 pre-existing failures unrelated to this change)

Closes #228

@TKaltofen TKaltofen requested a review from TomKaltofen as a code owner March 22, 2026 20:15
@TKaltofen TKaltofen force-pushed the fix/property-mapping-list-valued-options branch from e544aef to 5301462 Compare March 23, 2026 16:14
@TKaltofen TKaltofen force-pushed the fix/property-mapping-list-valued-options branch from 24e15c1 to 692a1ca Compare March 24, 2026 06:33
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
@TKaltofen TKaltofen force-pushed the fix/property-mapping-list-valued-options branch from 692a1ca to 713e16c Compare March 24, 2026 06:39
Runs list-valued options through mloda.run_all() pipeline to verify
that the list-to-tuple conversion preserves element order. Two tests
use different column orderings with weighted sums to prove order
matters and is correctly maintained.
@TKaltofen TKaltofen force-pushed the fix/property-mapping-list-valued-options branch from 713e16c to 0ab42a5 Compare March 24, 2026 06:42
@TomKaltofen TomKaltofen merged commit 95e2cd4 into main Mar 24, 2026
3 checks passed
@TomKaltofen TomKaltofen deleted the fix/property-mapping-list-valued-options branch March 24, 2026 06:45
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.

PROPERTY_MAPPING does not support list-valued options

2 participants