Skip to content

fix: support generators in is_in (#3195)#3689

Open
LukeTheoJohnson wants to merge 1 commit into
narwhals-dev:mainfrom
LukeTheoJohnson:fix-is-in-generators
Open

fix: support generators in is_in (#3195)#3689
LukeTheoJohnson wants to merge 1 commit into
narwhals-dev:mainfrom
LukeTheoJohnson:fix-is-in-generators

Conversation

@LukeTheoJohnson

@LukeTheoJohnson LukeTheoJohnson commented Jun 15, 2026

Copy link
Copy Markdown

Closes #3195.

is_in says it accepts any iterable, but in practice only lists were really tested. It turns out lists, tuples, sets, and ranges all work fine everywhere — but passing a generator raises on polars (both eager and lazy) and dask, while quietly working on the others.

The reason is that a generator can only be read once. Most backends happen to copy the values into a list internally, so they're fine — but polars and dask receive the raw generator and either reject it or try to read it more than once.

The fix is to spot a one-shot iterator and turn it into a list before handing it off, at the two public entry points (Expr.is_in and Series.is_in). The check (iter(x) is x) is only ever true for generators and similar single-use iterators — lists, tuples, sets, ranges, and native Series/arrays are all left exactly as they are today, so nothing that already works changes. It also avoids a subtle case where a generator could get used up partway through on lazy or partitioned backends.

Added regression tests covering tuple, set, range, and generator inputs for both Expr and Series.


One design call worth flagging: the issue suggested doing list(other) per-backend (like duckdb already does). I went with one shared check at the public layer instead — partly because polars is a passthrough with no backend file to patch, and partly to avoid repeating the same line across backends. Happy to switch to the per-backend approach if you'd prefer.

@LukeTheoJohnson LukeTheoJohnson marked this pull request as ready for review June 15, 2026 22:15
@FBruzzesi FBruzzesi added the suggest close Closing requires a second opinion label Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

suggest close Closing requires a second opinion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expr.is_in(Iterable) raises inconsistently

2 participants