fix: BaseFrame.filter with list[bool] in predicates

[FBruzzesi]

What type of PR is this? (check all applicable)

• 💾 Refactor
• ✨ Feature
• 🐛 Bug Fix
• 🔧 Optimization
• 📝 Documentation
• ✅ Test
• 🐳 Other

Related issues

• Closes Bug: DataFrame.filter silently ignores **constraints when using list[bool] #3182

Checklist

• Code follows style guide (ruff)
• Tests added
• Documented the changes

If you have comments or can explain your changes, please do so below

[dangotbanned]

You're more than welcome to steal these test cases 😉

• test_filter_mask_mixed

I was pretty surprised at how short the test suite was for filter_test.py

[FBruzzesi]

This was not covering full spectrum of possibilities

[dangotbanned]

• if any(is_list_of(pred, bool) for pred in predicates):

• tuple(plx._series.from_iterable(pred, context=plx).to_narwhals() if is_list_of(pred, bool) else pred for pred in predicates)

Move whatever is going on here into a new guard function like those in _utils.

Any time the logic starts getting hard to follow, TypeIs will save you 😉

Edit: I don't recommend putting everything in one method like this, but the polars logic has a few tricks to draw from

https://github.com/pola-rs/polars/blob/faab1f8eeeb1ad1a60fbd6e433bfbc3239737422/py-polars/src/polars/lazyframe/frame.py#L4074-L4143

[dangotbanned]

@FBruzzesi I just added more tests from upstream:

• test: Add some cases from polars

The last 2 would cover (df7acdd#r167422933)

[MarcoGorelli]

thanks both!

[dangotbanned]

@MarcoGorelli just to clarify (#3183 (comment)) and feel free to ignore if it was clear already

I somehow managed to start a thread that isn't showing on the PR 😭

• df7acdd#r167422933

And the commit I linked (which has more tests) is from (#3173).

I'm just trying to avoid the same bug I fixed in (pola-rs/polars#16254) from being introduced here 🙂

[FBruzzesi]

@dangotbanned if you are happy with the state of this, let's merge :)

[dangotbanned]

@dangotbanned if you are happy with the state of this, let's merge :)

Will do one more once over for (#3183 (comment)), but we should be good to merge later 👍

I've gotten a bit distracted by #3188, which we might need to fix upstream

[dangotbanned]

Thanks @FBruzzesi, provided I've not broken an edge case on an old version in (#3183 (comment)) - looks ready to merge