Skip to content

refactor: Simplify combine_metadata #2836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 17, 2025
Merged

refactor: Simplify combine_metadata #2836

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb298dc
refactor: Simplify combine_metadata
FBruzzesi Jul 16, 2025
aae5a94
Merge branch 'main' into refactor/simplify-combine_metadata
FBruzzesi Jul 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 21 additions & 26 deletions narwhals/_expression_parsing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ def from_horizontal_op(cls, *exprs: IntoExpr) -> ExprMetadata:
)


def combine_metadata( # noqa: C901, PLR0912
def combine_metadata(
*args: IntoExpr | object | None,
str_as_lit: bool,
allow_multi_output: bool,
Expand All @@ -526,17 +526,17 @@ def combine_metadata( # noqa: C901, PLR0912
# result preserves length if at least one input does
result_preserves_length = False
# result is elementwise if all inputs are elementwise
result_is_not_elementwise = False
result_is_elementwise = True
# result is scalar-like if all inputs are scalar-like
result_is_not_scalar_like = False
result_is_scalar_like = True
# result is literal if all inputs are literal
result_is_not_literal = False
result_is_literal = True

for i, arg in enumerate(args): # noqa: PLR1702
for i, arg in enumerate(args):
if (isinstance(arg, str) and not str_as_lit) or is_series(arg):
result_preserves_length = True
result_is_not_scalar_like = True
result_is_not_literal = True
result_is_scalar_like = False
result_is_literal = False
elif is_expr(arg):
metadata = arg._metadata
if metadata.expansion_kind.is_multi_output():
Expand All @@ -549,24 +549,19 @@ def combine_metadata( # noqa: C901, PLR0912
)
raise MultiOutputExpressionError(msg)
if not to_single_output:
if i == 0:
result_expansion_kind = expansion_kind
else:
result_expansion_kind = result_expansion_kind & expansion_kind
result_expansion_kind = (
result_expansion_kind & expansion_kind
if i > 0
else expansion_kind
)

if metadata.has_windows:
result_has_windows = True
result_has_windows |= metadata.has_windows
result_n_orderable_ops += metadata.n_orderable_ops
if metadata.preserves_length:
result_preserves_length = True
if not metadata.is_elementwise:
result_is_not_elementwise = True
if not metadata.is_scalar_like:
result_is_not_scalar_like = True
if not metadata.is_literal:
result_is_not_literal = True
if metadata.is_filtration:
n_filtrations += 1
result_preserves_length |= metadata.preserves_length
result_is_elementwise &= metadata.is_elementwise
result_is_scalar_like &= metadata.is_scalar_like
result_is_literal &= metadata.is_literal
n_filtrations += int(metadata.is_filtration)
Comment on lines +552 to +564
Copy link
Member

@dangotbanned dangotbanned Jul 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty impressed by this 🥳

I hadn't seen a real world use-case for __iand__ before, but this looks like it!


if n_filtrations > 1:
msg = "Length-changing expressions can only be used in isolation, or followed by an aggregation"
Expand All @@ -581,9 +576,9 @@ def combine_metadata( # noqa: C901, PLR0912
has_windows=result_has_windows,
n_orderable_ops=result_n_orderable_ops,
preserves_length=result_preserves_length,
is_elementwise=not result_is_not_elementwise,
is_scalar_like=not result_is_not_scalar_like,
is_literal=not result_is_not_literal,
is_elementwise=result_is_elementwise,
is_scalar_like=result_is_scalar_like,
is_literal=result_is_literal,
)


Expand Down
Loading