Skip to content

Conversation

@dangotbanned
Copy link
Member

@dangotbanned dangotbanned commented Oct 14, 2025

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

  • 🔧 Optimization

Related issues

Checklist

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

Description

At least until switching over to (pola-rs/polars#23351), this is a simple optimization that takes advantage of defining a class hierarchy 😄

The method was originally a port of some upstream code, but skipped the pl.col(pl.DataType) and struct/field stuff which we don't use:

@dangotbanned
Copy link
Member Author

@FBruzzesi thought I'd ping you as this part of the code reminded me of #2836

Was curious if you saw a way to do this in fewer steps? 😏

I think the only parts that would require some digging to know are

This branch is covering 3x subclasses:

elif isinstance(e, SelectorIR):

class RootSelector(SelectorIR):

class BinarySelector(

class InvertSelector(SelectorIR, t.Generic[SelectorT]):

But all the others are for these guys:

class _ColumnSelection(ExprIR, config=ExprIROptions.no_dispatch()):
"""Nodes which can resolve to `Column`(s) with a `Schema`."""
class Columns(_ColumnSelection):
__slots__ = ("names",)
names: Seq[str]
def __repr__(self) -> str:
return f"cols({list(self.names)!r})"
class Nth(_ColumnSelection):
__slots__ = ("index",)
index: int
def __repr__(self) -> str:
return f"nth({self.index})"
class IndexColumns(_ColumnSelection):
__slots__ = ("indices",)
indices: Seq[int]
def __repr__(self) -> str:
return f"index_columns({self.indices!r})"
class All(_ColumnSelection):
def __repr__(self) -> str:
return "all()"
class Exclude(_ColumnSelection, child=("expr",)):
__slots__ = ("expr", "names")
expr: ExprIR
"""Default is `all()`."""
names: Seq[str]
"""Excluded names."""
@staticmethod
def from_names(expr: ExprIR, *names: str | t.Iterable[str]) -> Exclude:
return Exclude(expr=expr, names=tuple(flatten_hash_safe(names)))
def __repr__(self) -> str:
return f"{self.expr!r}.exclude({list(self.names)!r})"

@dangotbanned dangotbanned marked this pull request as ready for review October 14, 2025 10:42
@dangotbanned dangotbanned merged commit fa0899a into oh-nodes Oct 15, 2025
31 of 32 checks passed
@dangotbanned dangotbanned deleted the expr-ir/opt-expansion branch October 15, 2025 18:01
@dangotbanned dangotbanned mentioned this pull request Oct 15, 2025
73 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants