feat: Implement list.contains for compatible types

[jonasdedden]

Daft's native list_contains requires exact type match (Int64 item vs Int32 list errors), so this uses expr.apply with Python in, which handles all dtypes and propagates nulls.

Requires #138 (narwhals pin update) first. The newly added exclusions in run_tests.py (categorical coverage etc.) come from the pin update, not from this change. Upstream added tests since the old pin that fail on main either way.

Verified green on the new pin. Ruff and pyright clean.

[FBruzzesi]

Thanks for the contribution @jonasdedden

Although I understand the request, for me we should reject implementations that require custom apply and UDF's. Those are very likely a performance killer, and we never want to be the bottleneck for a user.

I would instead consider opening a feature request in daft for a more generic list_contains support

[jonasdedden]

Thanks for the review @FBruzzesi !

A general question as I've never contributed to narwhals: what's preferred, support for as many expressions first, and focus on performance as a second step? Or instead only support those expressions that are very likely to give optimal/native performance and live with suboptimal expression support?

I'm not at home currently, but maybe something's possible to try to use Daft's native list_contains, and only fall back on a Python UDF on type mismatches?

[FBruzzesi]

A general question as I've never contributed to narwhals: what's preferred, support for as many expressions first, and focus on performance as a second step? Or instead only support those expressions that are very like to give optimal/native performance and live with suboptimal expression support?

No worries, that's something we might want to document better.
So far we have been implementing functionalities for a given backend only if that's doable with native operations without having to reach for python UDFs, materializing data, moving data between FFI; not even in the eager cases (pandas and pyarrow).

I personally think we should stick to that. We have been opening feature requests (and bugs) upstream in many many cases, asking for a feature to be supported natively, so this would not be a first time doing so.

I'm not at home currently, but maybe somethings possible to try to use Daft's native list_contains, and only fall back on a Python UDF on type mismatches?

That's definitely possible, but I would rather just wrap list_contains, document its limitations and possibly link a feature request issue, so that we know that when closed we are ready for full support behind a daft library version gate (or bump the min version for the plugin)

[jonasdedden]

The UDF is gone. For now I replaced it with a type-checking and then wrapping solution. The list's element dtype is resolved against the frame schema at plan time (a new DaftLazyFrame._native_dtype plus a frame-aware _with_elementwise_frame helper), and the literal is cast to it only when lossless: 1.5 or 300 against an Int8 list yield False rather than a truncated or null match, contains(None) uses the null count, and mismatched families (bool/str item on an int list) still raise like polars.

[MarcoGorelli]

thanks! could we just keep it simple and let Daft be as strict as it wants to be? (and/or raise a request with them to make it more relaxed?) i'd rather that than add a complex workaround here

[jonasdedden]

could we just keep it simple and let Daft be as strict as it wants to be?

For sure, but just so to check again, I had a look to what actually would be required to have full test compliance even with the not-yet-merged PRs narwhals-dev/narwhals#4001 and narwhals-dev/narwhals#3915 (which both add a few tests for list.contains stuff).

There were only 3 tiny commits (b296810 + 69b247d + c7f9dea, now called "fixes") required to have full compliance:

Test	PR 143 (previous status quo)	PR 143 + fixes	Simple wrap	Simple wrap + fixes
From #3915
test_contains_expr (Int32 list, item 2)	✅	✅	❌	❌
numeric_coercion[float_matches_int]	✅	✅	❌	❌
numeric_coercion[non_integer]	✅	✅	❌	❌
numeric_coercion[overflow]	✅	✅	❌	❌
numeric_coercion[int_matches_float]	✅	✅	❌	❌
invalid_item_raises[bool_item]	❌	✅	❌	✅
invalid_item_raises[str_item]	❌	✅	❌	✅
invalid_item_raises[datetime_precision]	❌	✅	❌	✅
test_contains_all_null_inner_expr	✅	✅	✅	✅
test_contains_no_match_with_null_elements_expr	✅	✅	❌	❌
test_contains_none_expr	✅	✅	✅	✅
test_contains_none_inner_dtypes_expr (String, nested List)	✅ ✅	✅ ✅	✅ ✅	✅ ✅
test_contains_none_single_empty_list_expr	✅	✅	✅	✅
New in #4001
test_contains_inner_dtypes_expr (String, Boolean, Date, NaN, ±inf)	✅ ×5	✅ ×5	✅ ×5	✅ ×5
test_contains_inner_dtypes_expr (Decimal)	❌	✅	❌	✅
Total	16 / 20	20 / 20	10 / 20	14 / 20

=> @MarcoGorelli I know you already pointed out you'd like a simple wrapper solution, but could you confirm once again that you want this (even with test compliance shortcomings)? 🙏 Sorry for the trouble!!! It's just that IMHO the logic in this PR is still relatively simple and we still could gradually remove it again once upstream issues are fixed.