Skip to content

Commit

Permalink
refactor(ir): remove find_first_base_table() analysis function (#8451)
Browse files Browse the repository at this point in the history
There were just a couple of places left where `find_first_base_table()`
was still in use. Since the epic split refactor the preferred way to
retrieve parent relation(s) is the `value.relations` property.
  • Loading branch information
kszucs authored Feb 27, 2024
1 parent b338517 commit 5cbd472
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 57 deletions.
23 changes: 0 additions & 23 deletions ibis/expr/analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,6 @@

import ibis.common.graph as g
import ibis.expr.operations as ops
from ibis.common.deferred import deferred, var
from ibis.common.patterns import pattern
from ibis.util import Namespace

p = Namespace(pattern, module=ops)
c = Namespace(deferred, module=ops)

x = var("x")
y = var("y")


# TODO(kszucs): should be removed
def find_first_base_table(node):
def predicate(node):
if isinstance(node, ops.Relation):
return g.halt, node
else:
return g.proceed, None

try:
return next(g.traverse(predicate, node))
except StopIteration:
return None


def flatten_predicates(node):
Expand Down
7 changes: 3 additions & 4 deletions ibis/expr/types/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -592,10 +592,9 @@ def to_torch(

def unbind(self) -> ir.Table:
"""Return an expression built on `UnboundTable` instead of backend-specific objects."""
from ibis.common.deferred import _
from ibis.expr.analysis import c, p
from ibis.expr.rewrites import _, d, p

rule = p.DatabaseTable >> c.UnboundTable(
rule = p.DatabaseTable >> d.UnboundTable(
name=_.name, schema=_.schema, namespace=_.namespace
)
return self.op().replace(rule).to_expr()
Expand All @@ -609,7 +608,7 @@ def as_table(self) -> ir.Table:
def as_scalar(self) -> ir.Scalar:
"""Convert an expression to a scalar."""
raise NotImplementedError(
f"{type(self)} expression cannot be converted into scalars"
f"{type(self)} expressions cannot be converted into scalars"
)


Expand Down
30 changes: 9 additions & 21 deletions ibis/expr/types/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1808,32 +1808,20 @@ def topk(
Table
A top-k expression
"""
from ibis.expr.types.relations import bind

from ibis.expr.analysis import find_first_base_table

arg_table = find_first_base_table(self.op()).to_expr()
try:
(table,) = self.op().relations
except ValueError:
raise com.IbisTypeError("TopK must depend on exactly one table.")

table = table.to_expr()
if by is None:
by = self.count()

if callable(by):
by = by(arg_table)
by_table = arg_table
elif isinstance(by, Value):
by_table = find_first_base_table(by.op()).to_expr()
metric = self.count()
else:
raise com.IbisTypeError(f"Invalid `by` argument with type {type(by)}")
(metric,) = bind(table, by)

assert by.op().name != self.op().name

if not arg_table.equals(by_table):
raise com.IbisError("Cross-table TopK; must provide a parent joined table")

return (
arg_table.aggregate(by, by=[self])
.order_by(ibis.desc(by.get_name()))
.limit(k)
)
return table.aggregate(metric, by=[self]).order_by(metric.desc()).limit(k)

def arbitrary(
self,
Expand Down
3 changes: 1 addition & 2 deletions ibis/expr/types/relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -4109,8 +4109,7 @@ def pivot_wider(
import pandas as pd

import ibis.selectors as s
from ibis import _
from ibis.expr.analysis import p, x
from ibis.expr.rewrites import _, p, x

orig_names_from = util.promote_list(names_from)

Expand Down
10 changes: 6 additions & 4 deletions ibis/expr/types/structs.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import ibis.expr.operations as ops
from ibis.common.deferred import deferrable
from ibis.common.exceptions import IbisError
from ibis.expr.types.generic import Column, Scalar, Value, literal

if TYPE_CHECKING:
Expand Down Expand Up @@ -336,11 +337,12 @@ def lift(self) -> ir.Table:
--------
[`Table.unpack`](./expression-tables.qmd#ibis.expr.types.relations.Table.unpack)
"""
import ibis.expr.analysis as an
try:
(table,) = self.op().relations
except ValueError:
raise IbisError("StructValue must depend on exactly one table")

# TODO(kszucs): avoid expression roundtripping
table = an.find_first_base_table(self.op()).to_expr()
return table[[self[name] for name in self.names]]
return table.to_expr().select([self[name] for name in self.names])

def destructure(self) -> list[ir.Value]:
"""Destructure a ``StructValue`` into the corresponding struct fields.
Expand Down
9 changes: 6 additions & 3 deletions ibis/selectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
from ibis import util
from ibis.common.collections import frozendict # noqa: TCH001
from ibis.common.deferred import Deferred, Resolver
from ibis.common.exceptions import IbisError
from ibis.common.grounds import Concrete, Singleton


Expand Down Expand Up @@ -622,10 +623,12 @@ def if_all(selector: Selector, predicate: Deferred | Callable) -> IfAnyAll:

class Sliceable(Singleton):
def __getitem__(self, key: str | int | slice | Iterable[int | str]) -> Predicate:
import ibis.expr.analysis as an

def pred(col: ir.Value) -> bool:
table = an.find_first_base_table(col.op())
try:
(table,) = col.op().relations
except ValueError:
raise IbisError("Column should depend on exactly one table")

schema = table.schema
idxs = schema._name_locs
num_names = len(schema)
Expand Down

0 comments on commit 5cbd472

Please sign in to comment.