Skip to content

Commit 90104e6

Browse files
authored
chore(expr-ir): Improve coverage (#3265)
1 parent 2a8212c commit 90104e6

34 files changed

+516
-232
lines changed

narwhals/_plan/_expansion.py

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,11 @@ def expand_selector_irs_names(
118118
ignored: Names of `group_by` columns.
119119
schema: Scope to expand selectors in.
120120
"""
121-
expander = Expander(schema, ignored)
122-
names = expander.iter_expand_selector_names(selectors)
123-
return _ensure_valid_output_names(tuple(names), expander.schema)
121+
names = tuple(Expander(schema, ignored).iter_expand_selector_names(selectors))
122+
if len(names) != len(set(names)):
123+
# NOTE: Can't easily reuse `duplicate_error`, falling back to main for now
124+
check_column_names_are_unique(names)
125+
return names
124126

125127

126128
def remove_alias(origin: ExprIR, /) -> ExprIR:
@@ -139,14 +141,6 @@ def fn(child: ExprIR, /) -> ExprIR:
139141
return origin.map_ir(fn)
140142

141143

142-
def _ensure_valid_output_names(names: Seq[str], schema: FrozenSchema) -> OutputNames:
143-
check_column_names_are_unique(names)
144-
output_names = names
145-
if not (set(schema.names).issuperset(output_names)):
146-
raise column_not_found_error(output_names, schema)
147-
return output_names
148-
149-
150144
class Expander:
151145
__slots__ = ("ignored", "schema")
152146
schema: FrozenSchema
@@ -223,7 +217,7 @@ def _expand_recursive(self, origin: ExprIR, /) -> Iterator[ExprIR]:
223217
yield from self._expand_function_expr(origin)
224218
else:
225219
msg = f"Didn't expect to see {type(origin).__name__}"
226-
raise TypeError(msg)
220+
raise NotImplementedError(msg)
227221

228222
def _expand_inner(self, children: Seq[ExprIR], /) -> Iterator[ExprIR]:
229223
"""Use when we want to expand non-root nodes, *without* duplicating the root.
@@ -265,8 +259,8 @@ def _expand_only(self, child: ExprIR, /) -> ExprIR:
265259
iterable = self._expand_recursive(child)
266260
first = next(iterable)
267261
if second := next(iterable, None):
268-
msg = f"Multi-output expressions are not supported in this context, got: `{second!r}`"
269-
raise MultiOutputExpressionError(msg)
262+
msg = f"Multi-output expressions are not supported in this context, got: `{second!r}`" # pragma: no cover
263+
raise MultiOutputExpressionError(msg) # pragma: no cover
270264
return first
271265

272266
# TODO @dangotbanned: It works, but all this class-specific branching belongs in the classes themselves

narwhals/_plan/_expr_ir.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ def iter_right(self) -> Iterator[ExprIR]:
152152
if isinstance(child, ExprIR):
153153
yield from child.iter_right()
154154
else:
155-
for node in reversed(child):
155+
for node in reversed(child): # pragma: no cover
156156
yield from node.iter_right()
157157

158158
def iter_root_names(self) -> Iterator[ExprIR]:
@@ -199,9 +199,8 @@ class SelectorIR(ExprIR, config=ExprIROptions.no_dispatch()):
199199
def to_narwhals(self, version: Version = Version.MAIN) -> Selector:
200200
from narwhals._plan.selectors import Selector, SelectorV1
201201

202-
if version is Version.MAIN:
203-
return Selector._from_ir(self)
204-
return SelectorV1._from_ir(self)
202+
tp = Selector if version is Version.MAIN else SelectorV1
203+
return tp._from_ir(self)
205204

206205
def into_columns(
207206
self, schema: FrozenSchema, ignored_columns: Container[str]
@@ -267,10 +266,10 @@ def map_ir(self, function: MapIR, /) -> Self:
267266
def __repr__(self) -> str:
268267
return f"{self.name}={self.expr!r}"
269268

270-
def _repr_html_(self) -> str:
269+
def _repr_html_(self) -> str: # pragma: no cover
271270
return f"<b>{self.name}</b>={self.expr._repr_html_()}"
272271

273-
def is_elementwise_top_level(self) -> bool:
272+
def is_elementwise_top_level(self) -> bool: # pragma: no cover
274273
"""Return True if the outermost node is elementwise.
275274
276275
Based on [`polars_plan::plans::aexpr::properties::AExpr.is_elementwise_top_level`]

narwhals/_plan/_guards.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,7 @@ def is_literal(obj: Any) -> TypeIs[ir.Literal[Any]]:
132132
return isinstance(obj, _ir().Literal)
133133

134134

135-
def is_horizontal_reduction(obj: Any) -> TypeIs[ir.FunctionExpr[Any]]:
136-
return is_function_expr(obj) and obj.options.is_input_wildcard_expansion()
137-
138-
139-
def is_tuple_of(obj: Any, tp: type[T]) -> TypeIs[Seq[T]]:
135+
# TODO @dangotbanned: Coverage
136+
# Used in `ArrowNamespace._vertical`, but only horizontal is covered
137+
def is_tuple_of(obj: Any, tp: type[T]) -> TypeIs[Seq[T]]: # pragma: no cover
140138
return bool(isinstance(obj, tuple) and obj and isinstance(obj[0], tp))

narwhals/_plan/_immutable.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ class Immutable(metaclass=ImmutableMeta):
3131
# NOTE: Trying to avoid this being added to synthesized `__init__`
3232
# Seems to be the only difference when decorating the metaclass
3333
__immutable_hash_value__: int
34+
else: # pragma: no cover
35+
...
3436

3537
__immutable_keys__: ClassVar[tuple[str, ...]]
3638

@@ -108,7 +110,9 @@ def __init__(self, **kwds: Any) -> None:
108110

109111
def _field_str(name: str, value: Any) -> str:
110112
if isinstance(value, tuple):
111-
inner = ", ".join(f"{v}" for v in value)
113+
inner = ", ".join(
114+
(f"{v!s}" if not isinstance(v, str) else f"{v!r}") for v in value
115+
)
112116
return f"{name}=[{inner}]"
113117
if isinstance(value, str):
114118
return f"{name}={value!r}"

narwhals/_plan/_parse.py

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,27 +6,23 @@
66
from itertools import chain
77
from typing import TYPE_CHECKING
88

9+
from narwhals._native import is_native_pandas
910
from narwhals._plan._guards import (
1011
is_column_name_or_selector,
1112
is_expr,
1213
is_into_expr_column,
1314
is_iterable_reject,
1415
is_selector,
1516
)
16-
from narwhals._plan.exceptions import (
17-
invalid_into_expr_error,
18-
is_iterable_pandas_error,
19-
is_iterable_polars_error,
20-
)
17+
from narwhals._plan.exceptions import invalid_into_expr_error, is_iterable_error
2118
from narwhals._utils import qualified_type_name
22-
from narwhals.dependencies import get_polars, is_pandas_dataframe, is_pandas_series
19+
from narwhals.dependencies import get_polars
2320
from narwhals.exceptions import InvalidOperationError
2421

2522
if TYPE_CHECKING:
2623
from collections.abc import Iterator
2724
from typing import Any, TypeVar
2825

29-
import polars as pl
3026
from typing_extensions import TypeAlias, TypeIs
3127

3228
from narwhals._plan.expr import Expr
@@ -126,7 +122,7 @@ def parse_into_expr_ir(
126122
expr = col(input)
127123
elif isinstance(input, list):
128124
if list_as_series is None:
129-
raise TypeError(input)
125+
raise TypeError(input) # pragma: no cover
130126
expr = lit(list_as_series(input))
131127
else:
132128
expr = lit(input, dtype=dtype)
@@ -140,9 +136,9 @@ def parse_into_selector_ir(input: ColumnNameOrSelector | Expr, /) -> SelectorIR:
140136
from narwhals._plan import selectors as cs
141137

142138
selector = cs.by_name(input)
143-
elif is_expr(input):
139+
elif is_expr(input): # pragma: no cover
144140
selector = input.meta.as_selector()
145-
else:
141+
else: # pragma: no cover
146142
msg = f"cannot turn {qualified_type_name(input)!r} into selector"
147143
raise TypeError(msg)
148144
return selector._ir
@@ -194,8 +190,8 @@ def _parse_sort_by_into_iter_expr_ir(
194190
) -> Iterator[ExprIR]:
195191
for e in _parse_into_iter_expr_ir(by, *more_by):
196192
if e.is_scalar:
197-
msg = f"All expressions sort keys must preserve length, but got:\n{e!r}"
198-
raise InvalidOperationError(msg)
193+
msg = f"All expressions sort keys must preserve length, but got:\n{e!r}" # pragma: no cover
194+
raise InvalidOperationError(msg) # pragma: no cover
199195
yield e
200196

201197

@@ -216,14 +212,14 @@ def _parse_into_iter_selector_ir(
216212

217213
if not _is_empty_sequence(first_input):
218214
if _is_iterable(first_input) and not isinstance(first_input, str):
219-
if more_inputs:
215+
if more_inputs: # pragma: no cover
220216
raise invalid_into_expr_error(first_input, more_inputs, {})
221217
else:
222218
for into in first_input: # type: ignore[var-annotated]
223219
yield parse_into_selector_ir(into)
224220
else:
225221
yield parse_into_selector_ir(first_input)
226-
for into in more_inputs:
222+
for into in more_inputs: # pragma: no cover
227223
yield parse_into_selector_ir(into)
228224

229225

@@ -298,18 +294,13 @@ def _combine_predicates(predicates: Iterator[ExprIR], /) -> ExprIR:
298294

299295

300296
def _is_iterable(obj: Iterable[T] | Any) -> TypeIs[Iterable[T]]:
301-
if is_pandas_dataframe(obj) or is_pandas_series(obj):
302-
raise is_iterable_pandas_error(obj)
303-
if _is_polars(obj):
304-
raise is_iterable_polars_error(obj)
297+
if is_native_pandas(obj) or (
298+
(pl := get_polars())
299+
and isinstance(obj, (pl.Series, pl.Expr, pl.DataFrame, pl.LazyFrame))
300+
):
301+
raise is_iterable_error(obj)
305302
return isinstance(obj, Iterable)
306303

307304

308305
def _is_empty_sequence(obj: Any) -> bool:
309306
return isinstance(obj, Sequence) and not obj
310-
311-
312-
def _is_polars(obj: Any) -> TypeIs[pl.Series | pl.Expr | pl.DataFrame | pl.LazyFrame]:
313-
return (pl := get_polars()) is not None and isinstance(
314-
obj, (pl.Series, pl.Expr, pl.DataFrame, pl.LazyFrame)
315-
)

narwhals/_plan/_rewrites.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ def map_ir(
8888
origin: NamedOrExprIRT, function: MapIR, *more_functions: MapIR
8989
) -> NamedOrExprIRT:
9090
"""Apply one or more functions, sequentially, to all of `origin`'s children."""
91-
if more_functions:
91+
if more_functions: # pragma: no cover
9292
result = origin
9393
for fn in (function, *more_functions):
9494
result = result.map_ir(fn)

narwhals/_plan/common.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131

3232
if sys.version_info >= (3, 13):
3333
from copy import replace as replace # noqa: PLC0414
34-
else:
34+
else: # pragma: no cover
3535

3636
def replace(obj: T, /, **changes: Any) -> T:
3737
cls = obj.__class__
@@ -98,20 +98,20 @@ def flatten_hash_safe(iterable: Iterable[OneOrIterable[T]], /) -> Iterator[T]:
9898
yield element # type: ignore[misc]
9999

100100

101-
def _not_one_or_iterable_str_error(obj: Any, /) -> TypeError:
101+
def _not_one_or_iterable_str_error(obj: Any, /) -> TypeError: # pragma: no cover
102102
msg = f"Expected one or an iterable of strings, but got: {qualified_type_name(obj)!r}\n{obj!r}"
103103
return TypeError(msg)
104104

105105

106106
def ensure_seq_str(obj: OneOrIterable[str], /) -> Seq[str]:
107107
if not isinstance(obj, Iterable):
108-
raise _not_one_or_iterable_str_error(obj)
108+
raise _not_one_or_iterable_str_error(obj) # pragma: no cover
109109
return (obj,) if isinstance(obj, str) else tuple(obj)
110110

111111

112112
def ensure_list_str(obj: OneOrIterable[str], /) -> list[str]:
113113
if not isinstance(obj, Iterable):
114-
raise _not_one_or_iterable_str_error(obj)
114+
raise _not_one_or_iterable_str_error(obj) # pragma: no cover
115115
return [obj] if isinstance(obj, str) else list(obj)
116116

117117

@@ -246,7 +246,7 @@ def _not_enough_room_error(cls, prefix: str, n_chars: int, /) -> NarwhalsError:
246246
available_chars = n_chars - len_prefix
247247
if available_chars < 0:
248248
visualize = ""
249-
else:
249+
else: # pragma: no cover (has coverage, but there's randomness in the test)
250250
okay = "✔" * available_chars
251251
bad = "✖" * (cls._MIN_RANDOM_CHARS - available_chars)
252252
visualize = f"\n Preview: '{prefix}{okay}{bad}'"

narwhals/_plan/dataframe.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ def version(self) -> Version:
5050
return self._version
5151

5252
@property
53-
def implementation(self) -> Implementation:
53+
def implementation(self) -> Implementation: # pragma: no cover
5454
return self._compliant.implementation
5555

5656
@property
@@ -61,7 +61,7 @@ def schema(self) -> Schema:
6161
def columns(self) -> list[str]:
6262
return self._compliant.columns
6363

64-
def __repr__(self) -> str: # pragma: no cover
64+
def __repr__(self) -> str:
6565
return generate_repr(f"nw.{type(self).__name__}", self.to_native().__repr__())
6666

6767
def __init__(self, compliant: CompliantFrame[Any, NativeFrameT_co], /) -> None:
@@ -70,12 +70,12 @@ def __init__(self, compliant: CompliantFrame[Any, NativeFrameT_co], /) -> None:
7070
def _with_compliant(self, compliant: CompliantFrame[Any, Incomplete], /) -> Self:
7171
return type(self)(compliant)
7272

73-
def to_native(self) -> NativeFrameT_co:
73+
def to_native(self) -> NativeFrameT_co: # pragma: no cover
7474
return self._compliant.native
7575

7676
def filter(
7777
self, *predicates: OneOrIterable[IntoExprColumn], **constraints: Any
78-
) -> Self:
78+
) -> Self: # pragma: no cover
7979
e = _parse.parse_predicates_constraints_into_expr_ir(*predicates, **constraints)
8080
named_irs, _ = prepare_projection((e,), schema=self)
8181
if len(named_irs) != 1:
@@ -113,7 +113,9 @@ def sort(
113113
def drop(self, *columns: str, strict: bool = True) -> Self:
114114
return self._with_compliant(self._compliant.drop(columns, strict=strict))
115115

116-
def drop_nulls(self, subset: str | Sequence[str] | None = None) -> Self:
116+
def drop_nulls(
117+
self, subset: str | Sequence[str] | None = None
118+
) -> Self: # pragma: no cover
117119
subset = [subset] if isinstance(subset, str) else subset
118120
return self._with_compliant(self._compliant.drop_nulls(subset))
119121

@@ -130,7 +132,7 @@ class DataFrame(
130132
def implementation(self) -> _EagerAllowedImpl:
131133
return self._compliant.implementation
132134

133-
def __len__(self) -> int:
135+
def __len__(self) -> int: # pragma: no cover
134136
return len(self._compliant)
135137

136138
@property
@@ -183,17 +185,17 @@ def to_dict(
183185
def to_dict(
184186
self, *, as_series: bool = True
185187
) -> dict[str, Series[NativeSeriesT]] | dict[str, list[Any]]:
186-
if as_series:
188+
if as_series: # pragma: no cover
187189
return {
188190
key: self._series(value)
189191
for key, value in self._compliant.to_dict(as_series=as_series).items()
190192
}
191193
return self._compliant.to_dict(as_series=as_series)
192194

193-
def to_series(self, index: int = 0) -> Series[NativeSeriesT]:
195+
def to_series(self, index: int = 0) -> Series[NativeSeriesT]: # pragma: no cover
194196
return self._series(self._compliant.to_series(index))
195197

196-
def get_column(self, name: str) -> Series[NativeSeriesT]:
198+
def get_column(self, name: str) -> Series[NativeSeriesT]: # pragma: no cover
197199
return self._series(self._compliant.get_column(name))
198200

199201
@overload
@@ -253,7 +255,7 @@ def filter(
253255
**constraints,
254256
)
255257
named_irs, _ = prepare_projection((e,), schema=self)
256-
if len(named_irs) != 1:
258+
if len(named_irs) != 1: # pragma: no cover
257259
# Should be unreachable, but I guess we will see
258260
msg = f"Expected a single predicate after expansion, but got {len(named_irs)!r}\n\n{named_irs!r}"
259261
raise ValueError(msg)

narwhals/_plan/exceptions.py

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from itertools import groupby
77
from typing import TYPE_CHECKING
88

9+
from narwhals._utils import qualified_type_name
910
from narwhals.exceptions import (
1011
ColumnNotFoundError,
1112
ComputeError,
@@ -21,9 +22,6 @@
2122
from collections.abc import Collection, Iterable
2223
from typing import Any
2324

24-
import pandas as pd
25-
import polars as pl
26-
2725
from narwhals._plan import expressions as ir
2826
from narwhals._plan._function import Function
2927
from narwhals._plan.expressions.operators import Operator
@@ -156,19 +154,9 @@ def invalid_into_expr_error(
156154
return InvalidIntoExprError(msg)
157155

158156

159-
def is_iterable_pandas_error(obj: pd.DataFrame | pd.Series[Any], /) -> TypeError:
160-
msg = (
161-
f"Expected Narwhals class or scalar, got: {type(obj)}. "
162-
"Perhaps you forgot a `nw.from_native` somewhere?"
163-
)
164-
return TypeError(msg)
165-
166-
167-
def is_iterable_polars_error(
168-
obj: pl.Series | pl.Expr | pl.DataFrame | pl.LazyFrame, /
169-
) -> TypeError:
157+
def is_iterable_error(obj: object, /) -> TypeError:
170158
msg = (
171-
f"Expected Narwhals class or scalar, got: {type(obj)}.\n\n"
159+
f"Expected Narwhals class or scalar, got: {qualified_type_name(obj)!r}.\n\n"
172160
"Hint: Perhaps you\n"
173161
"- forgot a `nw.from_native` somewhere?\n"
174162
"- used `pl.col` instead of `nw.col`?"

0 commit comments

Comments
 (0)