From 505801ea5469e08ed7cdd15127797f0fe1a373cf Mon Sep 17 00:00:00 2001 From: Hussain Sultan Date: Tue, 18 Aug 2026 11:11:40 -0400 Subject: [PATCH 1/7] refactor: move flavor helpers to _xorq, fold join_utils away get_ibis_module is the repo-wide ibis-flavor router (used by query, utils, ops, server, agents) and had nothing to do with nested-array compilation; importing it dragged nested_compile into every consumer. It now lives in _xorq.py (the bottom layer) together with _unwrap_table_proxy and null_safe_equal; the 14-line join_utils module is folded away. nested_compile re-exports get_ibis_module so the documented import path keeps working. Co-Authored-By: Claude Fable 5 --- src/boring_semantic_layer/_xorq.py | 44 +++++++++++++++++++++ src/boring_semantic_layer/join_utils.py | 14 ------- src/boring_semantic_layer/nested_compile.py | 34 +--------------- src/boring_semantic_layer/ops.py | 2 +- src/boring_semantic_layer/query.py | 2 +- 5 files changed, 47 insertions(+), 49 deletions(-) delete mode 100644 src/boring_semantic_layer/join_utils.py diff --git a/src/boring_semantic_layer/_xorq.py b/src/boring_semantic_layer/_xorq.py index 90e724a3..a5800fb9 100644 --- a/src/boring_semantic_layer/_xorq.py +++ b/src/boring_semantic_layer/_xorq.py @@ -15,6 +15,8 @@ from __future__ import annotations +import ibis as _plain_ibis + try: import xorq.api as api from xorq.api import selectors @@ -266,4 +268,46 @@ def walk_nodes(*args, **kwargs): "to_node", "types", "walk_nodes", + "get_ibis_module", + "null_safe_equal", ] + + +def _unwrap_table_proxy(obj): + for _ in range(8): + if not type(obj).__module__.startswith("boring_semantic_layer"): + return obj + inner = getattr(obj, "_t", None) + if inner is None: + resolver = getattr(obj, "_resolver", None) + inner = getattr(resolver, "_t", None) if resolver is not None else None + if inner is None: + return obj + obj = inner + return obj + + +def get_ibis_module(table): + """Return the ibis module that built ``table`` (regular vs xorq-vendored). + + BSL coexists with both flavors of ibis. Picking the right module avoids + cross-flavor literal/struct construction errors. Filter and dimension + callables receive resolver proxies rather than the table itself, so + unwrap those first — otherwise flavor detection would report plain ibis + for a xorq-backed table. + """ + table = _unwrap_table_proxy(table) + if type(table).__module__.startswith("xorq.vendor.ibis"): + return ibis + return _plain_ibis + + +def null_safe_equal(left, right): + """Return equality that also matches two NULL values. + + xorq/DataFusion currently misplans multiple ``identical_to`` join + predicates by folding an integer key into a boolean ``AND``. Expressing + the same semantics with ordinary equality and explicit NULL checks keeps + multi-key joins portable across the plain-ibis and xorq backends. + """ + return (left == right) | (left.isnull() & right.isnull()) diff --git a/src/boring_semantic_layer/join_utils.py b/src/boring_semantic_layer/join_utils.py deleted file mode 100644 index 94040479..00000000 --- a/src/boring_semantic_layer/join_utils.py +++ /dev/null @@ -1,14 +0,0 @@ -"""Small helpers shared by semantic join compilation paths.""" - -from __future__ import annotations - - -def null_safe_equal(left, right): - """Return equality that also matches two NULL values. - - xorq/DataFusion currently misplans multiple ``identical_to`` join - predicates by folding an integer key into a boolean ``AND``. Expressing - the same semantics with ordinary equality and explicit NULL checks keeps - multi-key joins portable across the plain-ibis and xorq backends. - """ - return (left == right) | (left.isnull() & right.isnull()) diff --git a/src/boring_semantic_layer/nested_compile.py b/src/boring_semantic_layer/nested_compile.py index 5b3fb0d8..cec9b3ca 100644 --- a/src/boring_semantic_layer/nested_compile.py +++ b/src/boring_semantic_layer/nested_compile.py @@ -19,39 +19,7 @@ import ibis from toolz import curry -from .join_utils import null_safe_equal - - -def get_ibis_module(table): - """Return the ibis module that built ``table`` (regular vs xorq-vendored). - - BSL coexists with both flavors of ibis. Picking the right module avoids - cross-flavor literal/struct construction errors. Filter and dimension - callables receive resolver proxies rather than the table itself, so - unwrap those first — otherwise flavor detection would report plain ibis - for a xorq-backed table. - """ - table = _unwrap_table_proxy(table) - table_module = type(table).__module__ - if table_module.startswith("xorq.vendor.ibis"): - from ._xorq import ibis as xorq_ibis - - return xorq_ibis - return ibis - - -def _unwrap_table_proxy(obj): - for _ in range(8): - if not type(obj).__module__.startswith("boring_semantic_layer"): - return obj - inner = getattr(obj, "_t", None) - if inner is None: - resolver = getattr(obj, "_resolver", None) - inner = getattr(resolver, "_t", None) if resolver is not None else None - if inner is None: - return obj - obj = inner - return obj +from ._xorq import get_ibis_module, null_safe_equal def _allocate_nested_array_name(table, idx: int) -> str: diff --git a/src/boring_semantic_layer/ops.py b/src/boring_semantic_layer/ops.py index aad36b89..6bea900f 100644 --- a/src/boring_semantic_layer/ops.py +++ b/src/boring_semantic_layer/ops.py @@ -58,8 +58,8 @@ from .calc_compiler import ( compile_calc_measure as _compile_calc_measure_impl, ) +from ._xorq import null_safe_equal from .graph_utils import walk_nodes -from .join_utils import null_safe_equal from .measure_scope import ( ColumnScope, MeasureScope, diff --git a/src/boring_semantic_layer/query.py b/src/boring_semantic_layer/query.py index 73cf7fb6..1e59ca28 100644 --- a/src/boring_semantic_layer/query.py +++ b/src/boring_semantic_layer/query.py @@ -575,7 +575,7 @@ def compare_periods( ) -> Any: """Compare two time ranges and return current/previous/delta columns.""" from .api import to_semantic_table - from .join_utils import null_safe_equal + from ._xorq import null_safe_equal dimensions = list(dimensions or []) measures = list(measures or []) From 845302f8ead85dec6a97aca0319db56fef497ae1 Mon Sep 17 00:00:00 2001 From: Hussain Sultan Date: Tue, 18 Aug 2026 11:13:20 -0400 Subject: [PATCH 2/7] refactor: detach agents (and the package root) from the import SCC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit agents/tools.py imported from_yaml from the package root instead of .yaml — the root __init__ lazily imports agents, so that one line made agents.tools, agents.backends.langgraph, AND boring_semantic_layer's __init__ mutually reachable with the core knot. SCC: 23 -> 20 modules. Co-Authored-By: Claude Fable 5 --- src/boring_semantic_layer/agents/tools.py | 2 +- src/boring_semantic_layer/tests/test_import_graph.py | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/boring_semantic_layer/agents/tools.py b/src/boring_semantic_layer/agents/tools.py index 7f739f23..58ecc400 100644 --- a/src/boring_semantic_layer/agents/tools.py +++ b/src/boring_semantic_layer/agents/tools.py @@ -12,7 +12,7 @@ import ibis from langchain_core.tools import ToolException -from boring_semantic_layer import from_yaml +from boring_semantic_layer.yaml import from_yaml from boring_semantic_layer.agents.utils.chart_handler import generate_chart_with_data from boring_semantic_layer.agents.utils.prompts import load_prompt from boring_semantic_layer.utils import safe_eval diff --git a/src/boring_semantic_layer/tests/test_import_graph.py b/src/boring_semantic_layer/tests/test_import_graph.py index 752e2b30..31c5f54b 100644 --- a/src/boring_semantic_layer/tests/test_import_graph.py +++ b/src/boring_semantic_layer/tests/test_import_graph.py @@ -26,9 +26,6 @@ # The measured SCC as of 2026-08 (main @ 25c79a2). Shrink-only. KNOWN_SCC_MEMBERS = frozenset( { - "boring_semantic_layer", - "boring_semantic_layer.agents.backends.langgraph", - "boring_semantic_layer.agents.tools", "boring_semantic_layer.api", "boring_semantic_layer.chart", "boring_semantic_layer.chart.altair_chart", From fb3323299096f579b4b99819f62e59837dce2967 Mon Sep 17 00:00:00 2001 From: Hussain Sultan Date: Tue, 18 Aug 2026 11:14:42 -0400 Subject: [PATCH 3/7] refactor: detach the chart cluster from the import SCC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit expr.py imported .chart at module level — core depending on a presentation extra, and the edge that closed the cycle keeping all six chart modules in the SCC (chart.utils reaches down into ops, which lazily reaches expr). The .chart() method now resolves the chart package at call time, pandas-.plot style. Core import no longer loads chart at all. SCC: 20 -> 14 modules. Co-Authored-By: Claude Fable 5 --- src/boring_semantic_layer/expr.py | 7 ++++++- src/boring_semantic_layer/tests/test_import_graph.py | 6 ------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/boring_semantic_layer/expr.py b/src/boring_semantic_layer/expr.py index 5a05178b..446750f6 100644 --- a/src/boring_semantic_layer/expr.py +++ b/src/boring_semantic_layer/expr.py @@ -18,7 +18,6 @@ GroupedTable, Table, ) -from .chart import chart as create_chart from .measure_scope import MeasureScope from .ops import ( Dimension, @@ -226,6 +225,12 @@ def chart( format: str = "static", ): """Create a chart from this semantic result.""" + # The chart package is a presentation extra layered ABOVE the core + # expression API; resolve it at call time so core never depends on + # it at import time (mirrors pandas-style optional .plot accessors). + import importlib + + create_chart = importlib.import_module("boring_semantic_layer.chart").chart return create_chart(self, spec=spec, backend=backend, format=format) def filter(self, predicate: Callable) -> SemanticFilter: diff --git a/src/boring_semantic_layer/tests/test_import_graph.py b/src/boring_semantic_layer/tests/test_import_graph.py index 31c5f54b..780a2dfd 100644 --- a/src/boring_semantic_layer/tests/test_import_graph.py +++ b/src/boring_semantic_layer/tests/test_import_graph.py @@ -27,12 +27,6 @@ KNOWN_SCC_MEMBERS = frozenset( { "boring_semantic_layer.api", - "boring_semantic_layer.chart", - "boring_semantic_layer.chart.altair_chart", - "boring_semantic_layer.chart.echarts_adapter", - "boring_semantic_layer.chart.plotext_chart", - "boring_semantic_layer.chart.plotly_chart", - "boring_semantic_layer.chart.utils", "boring_semantic_layer.convert", "boring_semantic_layer.expr", "boring_semantic_layer.format", From 68b4f5f1ae9826f1ec77017f1373606f07b165e6 Mon Sep 17 00:00:00 2001 From: Hussain Sultan Date: Tue, 18 Aug 2026 11:16:43 -0400 Subject: [PATCH 4/7] refactor: delete convert.py's dead ibis.expr.sql handlers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 11 @convert.register handlers were a parallel, drifted copy of the real to_untagged lowering with no caller anywhere in src or tests (the module was imported by __init__ solely 'to register dispatch handlers'). Only the resolver proxies (_Resolver, _AggResolver, _PrefixProxy) are used — convert.py shrinks 496 -> 117 lines, loses its ops import, and leaves the import SCC. SCC: 14 -> 13 modules. Co-Authored-By: Claude Fable 5 --- src/boring_semantic_layer/__init__.py | 7 +- src/boring_semantic_layer/convert.py | 388 +----------------- .../tests/test_import_graph.py | 1 - 3 files changed, 7 insertions(+), 389 deletions(-) diff --git a/src/boring_semantic_layer/__init__.py b/src/boring_semantic_layer/__init__.py index 4bcbf4ad..de107031 100644 --- a/src/boring_semantic_layer/__init__.py +++ b/src/boring_semantic_layer/__init__.py @@ -2,11 +2,8 @@ Semantic API layer on top of external ibis. """ -# Import convert and format to register dispatch handlers for semantic operations -from . import ( - convert, # noqa: F401 - format, # noqa: F401 -) +# Import format to register repr dispatch handlers for semantic operations +from . import format # noqa: F401 # Main API exports from .api import ( diff --git a/src/boring_semantic_layer/convert.py b/src/boring_semantic_layer/convert.py index fac8756f..0c985694 100644 --- a/src/boring_semantic_layer/convert.py +++ b/src/boring_semantic_layer/convert.py @@ -1,52 +1,17 @@ -"""Conversion functions for lowering semantic layer operations to Ibis. +"""Dimension-resolving table proxies used by filter/join/aggregate lowering. -This module contains all the converters that register with ibis.expr.sql.convert -to transform semantic layer operations into executable Ibis expressions. +Historically this module also registered ``ibis.expr.sql.convert`` handlers +for every semantic op — a parallel, drifted copy of the real ``to_untagged`` +lowering in ``ops.py`` with no remaining caller. Those handlers are gone; +only the resolver proxies survive. """ from __future__ import annotations from collections.abc import Callable -from typing import Protocol, runtime_checkable -import ibis from attrs import field, frozen -from ibis.common.collections import FrozenOrderedDict from ibis.expr import types as ir -from ibis.expr.sql import convert - -from boring_semantic_layer.ops import ( - SemanticAggregateOp, - SemanticFilterOp, - SemanticGroupByOp, - SemanticJoinOp, - SemanticLimitOp, - SemanticOrderByOp, - SemanticProjectOp, - SemanticTableOp, - SemanticUnnestOp, - _find_all_root_models, -) - -IbisTableExpr = ibis.expr.api.Table -IbisProject = ibis.expr.operations.relations.Project - - -@runtime_checkable -class AnyTable(Protocol): - """Protocol for table-like objects supporting column access. - - This protocol describes objects that provide table column access - through attribute and item notation, returning Ibis column expressions. - - Satisfied by: - - ir.Table: Direct Ibis tables - - _Resolver: Proxy with dimension resolution - - _AggResolver: Proxy for aggregation contexts - """ - - def __getattr__(self, name: str) -> ir.Value: ... - def __getitem__(self, name: str) -> ir.Value: ... class _PrefixProxy: @@ -150,346 +115,3 @@ def __getitem__(self, key: str): if key in self._meas: return self._meas[key](self._t) raise - - -# ============================================================================ -# Ibis converters (passthrough for standard Ibis operations) -# ============================================================================ - - -@convert.register(IbisTableExpr) -def _convert_ibis_table(expr, catalog, *args): - """Convert Ibis table expression to catalog form.""" - return convert(expr.op(), catalog=catalog) - - -@convert.register(IbisProject) -def _convert_ibis_project(op: IbisProject, catalog, *args): - """Convert Ibis project operation.""" - tbl = convert(op.parent, catalog=catalog) - cols = [v.to_expr().name(k) for k, v in op.values.items()] - return tbl.select(cols) - - -# ============================================================================ -# Helper functions for experimental nested access -# ============================================================================ - - -def _process_nested_access_marker(marker, table): - """Convert NestedAccessMarker to actual Ibis expression with unnesting. - - Args: - marker: NestedAccessMarker indicating what unnesting is needed - table: Base Ibis table - - Returns: - Tuple of (unnested_table, ibis_expression) - """ - from boring_semantic_layer.nested_access import NestedAccessMarker - - if not isinstance(marker, NestedAccessMarker): - return (table, marker) - - # Unnest all array columns in the path - unnested_tbl = table - for array_col in marker.array_path: - if array_col in unnested_tbl.columns: - unnested_tbl = unnested_tbl.unnest(array_col) - - # Build expression accessing nested fields - if marker.field_path: - # Access the unnested array column (which is now a struct) - expr = getattr(unnested_tbl, marker.array_path[0]) - # Navigate through struct fields - for field in marker.field_path: - expr = getattr(expr, field) - else: - # No field path - operate on the whole unnested table - expr = unnested_tbl - - # Apply the aggregation operation - if marker.operation == "count": - return (unnested_tbl, unnested_tbl.count()) - elif marker.operation == "sum": - return (unnested_tbl, expr.sum()) - elif marker.operation == "mean": - return (unnested_tbl, expr.mean()) - elif marker.operation == "min": - return (unnested_tbl, expr.min()) - elif marker.operation == "max": - return (unnested_tbl, expr.max()) - elif marker.operation == "nunique": - return (unnested_tbl, expr.nunique()) - else: - raise ValueError(f"Unknown nested access operation: {marker.operation}") - - -def _evaluate_measure_with_nested_access(measure_fn, table): - """Evaluate a measure function, detecting and handling NestedAccessMarkers. - - Args: - measure_fn: Measure function (callable) - table: Base Ibis table - - Returns: - Tuple of (unnested_table_or_none, ibis_expression) - If unnested_table is not None, the measure required unnesting - """ - from boring_semantic_layer.nested_access import NestedAccessMarker - - # Call the measure function - result = measure_fn(table) - - # Check if it returned a NestedAccessMarker - if isinstance(result, NestedAccessMarker): - return _process_nested_access_marker(result, table) - else: - return (None, result) - - -# ============================================================================ -# Semantic layer converters -# ============================================================================ - - -@convert.register(SemanticTableOp) -def _convert_semantic_table(node: SemanticTableOp, catalog, *args): - """Convert SemanticTableOp to base Ibis table.""" - return convert(node.table, catalog=catalog) - - -@convert.register(SemanticFilterOp) -def _convert_semantic_filter(node: SemanticFilterOp, catalog, *args): - """Convert SemanticFilterOp to Ibis filter. - - Resolves dimension references in the filter predicate and applies - the filter to the base table. - """ - from boring_semantic_layer.ops import ( - SemanticAggregateOp, - _augment_dimensions_with_raw_columns, - _exact_filter_fields, - _get_merged_fields, - _unwrap, - _validate_qualified_filter_fields, - ) - - all_roots = _find_all_root_models(node.source) - base_tbl = convert(node.source, catalog=catalog) - - pred_fn = _unwrap(node.predicate) - exact_fields = _exact_filter_fields(pred_fn) - dim_map = ( - {} - if isinstance(node.source, SemanticAggregateOp) - else _get_merged_fields( - all_roots, - "dimensions", - source=node.source, - ) - ) - if not isinstance(node.source, SemanticAggregateOp) and exact_fields: - dim_map = _augment_dimensions_with_raw_columns( - dim_map, - exact_fields, - all_roots, - node.source, - ) - _validate_qualified_filter_fields(exact_fields, dim_map, all_roots) - pred = pred_fn(_Resolver(base_tbl, dim_map)) - return base_tbl.filter(pred) - - -@convert.register(SemanticProjectOp) -def _convert_semantic_project(node: SemanticProjectOp, catalog, *args): - """Convert SemanticProjectOp to Ibis select/aggregate. - - Handles projection of: - - Dimensions (potentially with aggregation if measures are also selected) - - Measures (triggers aggregation) - - Raw table columns - - Experimental: Automatic unnesting for NestedAccessMarker results - """ - from boring_semantic_layer.ops import _get_merged_fields - - all_roots = _find_all_root_models(node.source) - tbl = convert(node.source, catalog=catalog) - - if not all_roots: - return tbl.select([getattr(tbl, f) for f in node.fields]) - - merged_dimensions = _get_merged_fields(all_roots, "dimensions") - merged_measures = _get_merged_fields(all_roots, "measures") - - dims = [f for f in node.fields if f in merged_dimensions] - meas = [f for f in node.fields if f in merged_measures] - raw_fields = [f for f in node.fields if f not in merged_dimensions and f not in merged_measures] - - # Evaluate dimension expressions - dim_exprs = [merged_dimensions[name](tbl).name(name) for name in dims] - - # Evaluate measure expressions, checking for NestedAccessMarkers - meas_exprs = [] - unnested_tbl = tbl # Track if we need to unnest the table - needs_unnesting = False - - for name in meas: - unnested, expr = _evaluate_measure_with_nested_access(merged_measures[name], tbl) - if unnested is not None: - # This measure needs unnesting - use the unnested table - unnested_tbl = unnested - needs_unnesting = True - meas_exprs.append(expr.name(name)) - - # Use unnested table if any measure needed it - active_tbl = unnested_tbl if needs_unnesting else tbl - - # Re-evaluate dimensions on unnested table if needed - if needs_unnesting and dim_exprs: - dim_exprs = [merged_dimensions[name](active_tbl).name(name) for name in dims] - - raw_exprs = [getattr(active_tbl, name) for name in raw_fields if hasattr(active_tbl, name)] - - return ( - active_tbl.group_by(dim_exprs).aggregate(meas_exprs) - if meas_exprs and dim_exprs - else active_tbl.aggregate(meas_exprs) - if meas_exprs - else active_tbl.select(dim_exprs + raw_exprs) - if dim_exprs or raw_exprs - else active_tbl - ) - - -@convert.register(SemanticGroupByOp) -def _convert_semantic_groupby(node: SemanticGroupByOp, catalog, *args): - """Convert SemanticGroupByOp (passthrough - grouping happens in aggregate).""" - return convert(node.source, catalog=catalog) - - -@convert.register(SemanticJoinOp) -def _convert_semantic_join(node: SemanticJoinOp, catalog, *args): - """Convert SemanticJoinOp to Ibis join. - - Handles both conditional joins (with ON clause) and cross joins. - Resolves dimensions from both left and right tables for the join condition. - """ - left_tbl = convert(node.left, catalog=catalog) - right_tbl = convert(node.right, catalog=catalog) - - if node.on is not None: - # Get dimensions from left and right for semantic resolution - left_dims = {k: v.expr for k, v in node.left.get_dimensions().items()} - right_dims = {k: v.expr for k, v in node.right.get_dimensions().items()} - - return left_tbl.join( - right_tbl, - node.on(_Resolver(left_tbl, left_dims), _Resolver(right_tbl, right_dims)), - how=node.how, - ) - else: - return left_tbl.join(right_tbl, how=node.how) - - -@convert.register(SemanticAggregateOp) -def _convert_semantic_aggregate(node: SemanticAggregateOp, catalog, *args): - """Convert SemanticAggregateOp to Ibis group_by + aggregate. - - Resolves: - - Group by keys (dimensions or raw columns) - - Aggregation expressions (measures) - - Returns aggregated table with properly named columns. - """ - from boring_semantic_layer.ops import _get_merged_fields - - all_roots = _find_all_root_models(node.source) - tbl = convert(node.source, catalog=catalog) - - merged_dimensions = _get_merged_fields(all_roots, "dimensions") - merged_measures = _get_merged_fields(all_roots, "measures") - - group_exprs = [ - (merged_dimensions[k](tbl).name(k) if k in merged_dimensions else getattr(tbl, k).name(k)) - for k in node.keys - ] - - proxy = _AggResolver(tbl, merged_dimensions, merged_measures) - meas_exprs = [fn(proxy).name(name) for name, fn in node.aggs.items()] - metrics = FrozenOrderedDict({expr.get_name(): expr for expr in meas_exprs}) - - return tbl.group_by(group_exprs).aggregate(metrics) if group_exprs else tbl.aggregate(metrics) - - -@convert.register(SemanticOrderByOp) -def _convert_semantic_orderby(node: SemanticOrderByOp, catalog, *args): - """Convert SemanticOrderByOp to Ibis order_by. - - Handles: - - String keys (column names) - - Deferred expressions (from lambda functions) - - Direct column references - """ - tbl = convert(node.source, catalog=catalog) - - def resolve_key(key): - return ( - getattr(tbl, key) - if hasattr(tbl, key) - else tbl[key] - if isinstance(key, str) and key in tbl.columns - else key[1](tbl) - if isinstance(key, tuple) and len(key) == 2 and key[0] == "__deferred__" - else key - ) - - return tbl.order_by([resolve_key(key) for key in node.keys]) - - -@convert.register(SemanticLimitOp) -def _convert_semantic_limit(node: SemanticLimitOp, catalog, *args): - """Convert SemanticLimitOp to Ibis limit. - - Applies row limit with optional offset. - """ - tbl = convert(node.source, catalog=catalog) - return tbl.limit(node.n) if node.offset == 0 else tbl.limit(node.n, offset=node.offset) - - -@convert.register(SemanticUnnestOp) -def _convert_semantic_unnest(node: SemanticUnnestOp, catalog, *args): - """Convert SemanticUnnestOp to Ibis unnest. - - Expands array column into separate rows, optionally unpacking struct fields. - """ - - def build_struct_fields(col_expr, col_type): - """Pure function: build dict of struct field selections.""" - return {name: col_expr[name] for name in col_type.names} - - def unpack_struct_if_needed(unnested_tbl, column_name): - """Conditionally unpack struct fields into top-level columns.""" - if column_name not in unnested_tbl.columns: - return unnested_tbl - - col_expr = unnested_tbl[column_name] - col_type = col_expr.type() - - if hasattr(col_type, "fields") and col_type.fields: - struct_fields = build_struct_fields(col_expr, col_type) - return unnested_tbl.select(unnested_tbl, **struct_fields) - - return unnested_tbl - - tbl = convert(node.source, catalog=catalog) - - if node.column not in tbl.columns: - raise ValueError(f"Column '{node.column}' not found in table") - - try: - unnested = tbl.unnest(node.column) - except Exception as e: - raise ValueError(f"Failed to unnest column '{node.column}': {e}") from e - - return unpack_struct_if_needed(unnested, node.column) diff --git a/src/boring_semantic_layer/tests/test_import_graph.py b/src/boring_semantic_layer/tests/test_import_graph.py index 780a2dfd..60533127 100644 --- a/src/boring_semantic_layer/tests/test_import_graph.py +++ b/src/boring_semantic_layer/tests/test_import_graph.py @@ -27,7 +27,6 @@ KNOWN_SCC_MEMBERS = frozenset( { "boring_semantic_layer.api", - "boring_semantic_layer.convert", "boring_semantic_layer.expr", "boring_semantic_layer.format", "boring_semantic_layer.graph_utils", From 93e38436b45c96267b06b1b188d649693103c1fa Mon Sep 17 00:00:00 2001 From: Hussain Sultan Date: Tue, 18 Aug 2026 11:18:42 -0400 Subject: [PATCH 5/7] refactor: delete api.py's unexported functional wrappers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit join_one/join_many/join_cross/filter_/group_by_/aggregate_/mutate_/ order_by_/limit_ were pure pass-throughs to the fluent methods — never exported from __init__, never documented, used by exactly one test (now rewritten fluent-style). api.py: 279 -> 124 lines; what remains (to_semantic_table, entity_dimension, time_dimension) is all public. Co-Authored-By: Claude Fable 5 --- src/boring_semantic_layer/api.py | 164 +----------------- .../tests/test_stale_dimensions.py | 13 +- 2 files changed, 7 insertions(+), 170 deletions(-) diff --git a/src/boring_semantic_layer/api.py b/src/boring_semantic_layer/api.py index a7df097f..e7ec8791 100644 --- a/src/boring_semantic_layer/api.py +++ b/src/boring_semantic_layer/api.py @@ -6,11 +6,10 @@ from __future__ import annotations -from collections.abc import Callable, Sequence -from typing import TYPE_CHECKING, Any +from collections.abc import Callable +from typing import TYPE_CHECKING if TYPE_CHECKING: - from ibis.common.deferred import Deferred from ibis.expr import types as ir from .expr import SemanticModel @@ -44,165 +43,6 @@ def to_semantic_table( ) -def join_one( - left: SemanticModel, - other: SemanticModel, - on: Callable[[Any, Any], ir.BooleanValue] | str | Deferred | Sequence[str | Deferred], - how: str = "left", -) -> SemanticModel: - """Join two semantic tables with a one-to-one relationship (left outer join). - - Args: - left: Left semantic table - other: Right semantic table - on: Join predicate. Accepts a lambda ``(left, right) -> bool``, a column - name string, a Deferred ``_.col``, or a list of strings/Deferred for - compound equi-joins. - how: Join type. Only ``"left"`` is supported. - - Returns: - Joined SemanticModel - - Examples: - >>> join_one(orders, customers, on="customer_id") - >>> join_one(orders, customers, on=_.customer_id) - >>> join_one(orders, customers, on=lambda o, c: o.customer_id == c.customer_id) - """ - return left.join_one(other, on, how) - - -def join_many( - left: SemanticModel, - other: SemanticModel, - on: Callable[[Any, Any], ir.BooleanValue] | str | Deferred | Sequence[str | Deferred], - how: str = "left", -) -> SemanticModel: - """Join two semantic tables with a one-to-many relationship. - - Args: - left: Left semantic table - other: Right semantic table - on: Join predicate. Accepts a lambda ``(left, right) -> bool``, a column - name string, a Deferred ``_.col``, or a list of strings/Deferred for - compound equi-joins. - how: Join type. Only ``"left"`` is supported. - - Returns: - Joined SemanticModel - - Examples: - >>> join_many(customer, orders, on="customer_id") - >>> join_many(customer, orders, on=_.customer_id) - >>> join_many(customer, orders, on=lambda c, o: c.customer_id == o.customer_id) - """ - return left.join_many(other, on, how) - - -def join_cross(left: SemanticModel, other: SemanticModel) -> SemanticModel: - """Cross join (Cartesian product) two semantic tables. - - Args: - left: Left semantic table - other: Right semantic table - - Returns: - Joined SemanticModel (Cartesian product of all rows) - - Examples: - >>> join_cross(table_a, table_b) # All combinations of rows - """ - return left.join_cross(other) - - -def filter_( - table: SemanticModel, - predicate: Callable[[ir.Table], ir.BooleanValue], -) -> SemanticModel: - """Filter a semantic table by a predicate. - - Args: - table: Semantic table to filter - predicate: Function that takes a table and returns a boolean expression - - Returns: - Filtered SemanticModel - """ - return table.filter(predicate) - - -def group_by_(table: SemanticModel, *dims: str | Deferred) -> SemanticModel: - """Group a semantic table by dimensions. - - Args: - table: Semantic table to group - *dims: Dimension names to group by - - Returns: - Grouped SemanticModel - """ - return table.group_by(*dims) - - -def aggregate_( - table: SemanticModel, - *measure_names: str | Callable | Deferred, - **aliased, -) -> SemanticModel: - """Aggregate measures in a semantic table. - - Args: - table: Semantic table to aggregate - *measure_names: Names of measures to aggregate - **aliased: Aliased measure aggregations - - Returns: - Aggregated SemanticModel - """ - return table.aggregate(*measure_names, **aliased) - - -def mutate_( - table: SemanticModel, - **kwargs: Callable[[ir.Table], ir.Value], -) -> SemanticModel: - """Add computed columns to a semantic table. - - Args: - table: Semantic table to mutate - **kwargs: Named column expressions (xorq vendored ibis expressions) - - Returns: - Mutated SemanticModel - """ - return table.mutate(**kwargs) - - -def order_by_(table: SemanticModel, *keys: str | ir.Value) -> SemanticModel: - """Order a semantic table by keys. - - Args: - table: Semantic table to order - *keys: Column names or expressions to order by - - Returns: - Ordered SemanticModel - """ - return table.order_by(*keys) - - -def limit_(table: SemanticModel, n: int) -> SemanticModel: - """Limit the number of rows in a semantic table. - - Args: - table: Semantic table to limit - n: Maximum number of rows - - Returns: - Limited SemanticModel - """ - return table.limit(n) - - def entity_dimension( expr: Callable[[ir.Table], ir.Value], description: str | None = None, diff --git a/src/boring_semantic_layer/tests/test_stale_dimensions.py b/src/boring_semantic_layer/tests/test_stale_dimensions.py index e74fd748..a39c2406 100644 --- a/src/boring_semantic_layer/tests/test_stale_dimensions.py +++ b/src/boring_semantic_layer/tests/test_stale_dimensions.py @@ -7,7 +7,6 @@ import pandas as pd from boring_semantic_layer import to_semantic_table -from boring_semantic_layer.api import aggregate_, group_by_, join_one def test_bracket_filter_after_join_and_aggregate(): @@ -63,12 +62,11 @@ def test_bracket_filter_after_join_and_aggregate(): country=lambda t: t.country, ) - step1 = join_one(model_a, model_b, lambda a, b: a.order_id == b.order_id) - step2 = aggregate_( - group_by_(step1, "orders.region", "orders.customer_id"), + step1 = model_a.join_one(model_b, lambda a, b: a.order_id == b.order_id) + step2 = step1.group_by("orders.region", "orders.customer_id").aggregate( lambda t: t["orders.order_count"], ) - final = join_one(step2, model_c, lambda s, c: s["orders.customer_id"] == c.customer_id) + final = step2.join_one(model_c, lambda s, c: s["orders.customer_id"] == c.customer_id) df = final.filter(lambda t: t["orders.region"] == "North").execute() assert df.shape[0] == 2 @@ -126,12 +124,11 @@ def test_filter_before_aggregation_on_joined_table(): ) # Join orders with customers - joined = join_one(orders_sm, customers_sm, lambda o, c: o.customer_id == c.customer_id) + joined = orders_sm.join_one(customers_sm, lambda o, c: o.customer_id == c.customer_id) # Filter THEN aggregate - this is the critical pattern that was broken filtered = joined.filter(lambda t: t.country == "US") - aggregated = aggregate_( - group_by_(filtered, "customers.name"), + aggregated = filtered.group_by("customers.name").aggregate( lambda t: t["orders.total_amount"], ) From 6bcafcf871f5024ad623b2ec0663a24876dcbc2b Mon Sep 17 00:00:00 2001 From: Hussain Sultan Date: Tue, 18 Aug 2026 11:21:06 -0400 Subject: [PATCH 6/7] refactor: delete the legacy lambda-to-string expression codec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit expr_to_ibis_string / ibis_string_to_expr and their five source- introspection helpers (~170 lines in utils.py) were the pre-v2 'expressions as strings' serialization path, superseded by the structured resolver-tree codec. No production caller remained — only tests exercising the dead feature, which are removed with it. Co-Authored-By: Claude Fable 5 --- src/boring_semantic_layer/agents/tools.py | 2 +- src/boring_semantic_layer/nested_compile.py | 5 +- src/boring_semantic_layer/ops.py | 2 +- src/boring_semantic_layer/query.py | 2 +- .../tests/test_flavor_routing.py | 24 --- src/boring_semantic_layer/tests/test_utils.py | 32 ---- .../tests/test_xorq_convert.py | 48 ----- src/boring_semantic_layer/utils.py | 177 +----------------- 8 files changed, 8 insertions(+), 284 deletions(-) diff --git a/src/boring_semantic_layer/agents/tools.py b/src/boring_semantic_layer/agents/tools.py index 58ecc400..d52f1ed5 100644 --- a/src/boring_semantic_layer/agents/tools.py +++ b/src/boring_semantic_layer/agents/tools.py @@ -12,10 +12,10 @@ import ibis from langchain_core.tools import ToolException -from boring_semantic_layer.yaml import from_yaml from boring_semantic_layer.agents.utils.chart_handler import generate_chart_with_data from boring_semantic_layer.agents.utils.prompts import load_prompt from boring_semantic_layer.utils import safe_eval +from boring_semantic_layer.yaml import from_yaml def _models_ibis_module(models: dict) -> Any: diff --git a/src/boring_semantic_layer/nested_compile.py b/src/boring_semantic_layer/nested_compile.py index cec9b3ca..26bca7df 100644 --- a/src/boring_semantic_layer/nested_compile.py +++ b/src/boring_semantic_layer/nested_compile.py @@ -16,10 +16,11 @@ from functools import reduce from typing import Any -import ibis from toolz import curry -from ._xorq import get_ibis_module, null_safe_equal +# Back-compat re-export: user-facing error messages document this import path. +from ._xorq import get_ibis_module as get_ibis_module # noqa: PLC0414 +from ._xorq import null_safe_equal def _allocate_nested_array_name(table, idx: int) -> str: diff --git a/src/boring_semantic_layer/ops.py b/src/boring_semantic_layer/ops.py index 6bea900f..5afb0ce0 100644 --- a/src/boring_semantic_layer/ops.py +++ b/src/boring_semantic_layer/ops.py @@ -22,6 +22,7 @@ from ._xorq import ( FrozenDict, FrozenOrderedDict, + null_safe_equal, ) from ._xorq import ( Schema as XorqSchema, @@ -58,7 +59,6 @@ from .calc_compiler import ( compile_calc_measure as _compile_calc_measure_impl, ) -from ._xorq import null_safe_equal from .graph_utils import walk_nodes from .measure_scope import ( ColumnScope, diff --git a/src/boring_semantic_layer/query.py b/src/boring_semantic_layer/query.py index 1e59ca28..131b944c 100644 --- a/src/boring_semantic_layer/query.py +++ b/src/boring_semantic_layer/query.py @@ -574,8 +574,8 @@ def compare_periods( limit: int | None = None, ) -> Any: """Compare two time ranges and return current/previous/delta columns.""" - from .api import to_semantic_table from ._xorq import null_safe_equal + from .api import to_semantic_table dimensions = list(dimensions or []) measures = list(measures or []) diff --git a/src/boring_semantic_layer/tests/test_flavor_routing.py b/src/boring_semantic_layer/tests/test_flavor_routing.py index 1ecac72b..f3ee86c6 100644 --- a/src/boring_semantic_layer/tests/test_flavor_routing.py +++ b/src/boring_semantic_layer/tests/test_flavor_routing.py @@ -214,27 +214,3 @@ def test_agent_query_with_module_literal(self, flights_table): out = result.execute() assert len(out) == 1 assert out["cnt"].iloc[0] == 2 - - -class TestIbisStringToExprFlavor: - """ibis_string_to_expr lambdas re-bind ``ibis`` to the flavor of the - table they are called with.""" - - def test_literal_expression_against_converted_table(self, flights_table): - from boring_semantic_layer.utils import ibis_string_to_expr - - sm = _flights_model(flights_table) - fn = ibis_string_to_expr("_.dep_delay >= ibis.literal(8.0)").unwrap() - resolved = fn(sm.table) - # Must be a real boolean expression of the table's own flavor, - # not a Python bool from identity comparison. - assert not isinstance(resolved, bool) - assert type(resolved).__module__.split(".")[0] == type(sm.table).__module__.split(".")[0] - - def test_literal_expression_against_plain_table(self, flights_table): - from boring_semantic_layer.utils import ibis_string_to_expr - - fn = ibis_string_to_expr("_.dep_delay >= ibis.literal(8.0)").unwrap() - resolved = fn(flights_table) - assert not isinstance(resolved, bool) - assert type(resolved).__module__.startswith("ibis.") diff --git a/src/boring_semantic_layer/tests/test_utils.py b/src/boring_semantic_layer/tests/test_utils.py index 1723774a..26e2acb6 100644 --- a/src/boring_semantic_layer/tests/test_utils.py +++ b/src/boring_semantic_layer/tests/test_utils.py @@ -6,8 +6,6 @@ from boring_semantic_layer.utils import ( _is_url, - expr_to_ibis_string, - ibis_string_to_expr, safe_eval, ) @@ -130,36 +128,6 @@ def test_safe_eval_ibis_complex_expression(): assert isinstance(result, Success) -def test_expr_to_ibis_string(): - fn = lambda t: t.distance.mean() # noqa: E731 - result = expr_to_ibis_string(fn) - assert isinstance(result, Success) - ibis_str = result.unwrap() - assert ibis_str == "_.distance.mean()" - - -def test_expr_to_ibis_string_simple(): - fn = lambda t: t.origin # noqa: E731 - result = expr_to_ibis_string(fn) - assert isinstance(result, Success) - ibis_str = result.unwrap() - assert ibis_str == "_.origin" - - -def test_ibis_string_to_expr(): - result = ibis_string_to_expr("_.distance.mean()") - assert isinstance(result, Success) - fn = result.unwrap() - assert callable(fn) - - -def test_ibis_string_to_expr_simple(): - result = ibis_string_to_expr("_.origin") - assert isinstance(result, Success) - fn = result.unwrap() - assert callable(fn) - - def test_no_file_access(): result = safe_eval("open('/etc/passwd')") assert isinstance(result, Failure) diff --git a/src/boring_semantic_layer/tests/test_xorq_convert.py b/src/boring_semantic_layer/tests/test_xorq_convert.py index 42c1a731..b2001dca 100644 --- a/src/boring_semantic_layer/tests/test_xorq_convert.py +++ b/src/boring_semantic_layer/tests/test_xorq_convert.py @@ -10,7 +10,6 @@ to_tagged, try_import_xorq, ) -from boring_semantic_layer.utils import expr_to_ibis_string, ibis_string_to_expr xorq = pytest.importorskip("xorq", reason="xorq not installed") @@ -25,53 +24,6 @@ def test_try_import_xorq(): assert hasattr(xorq_mod.api, "memtable") -def test_serialize_ibis_lambda(): - fn = lambda t: t.col1 # noqa: E731 - - result = expr_to_ibis_string(fn) - assert isinstance(result, Success | Failure) - - if isinstance(result, Success): - serialized = result.unwrap() - assert isinstance(serialized, str) - assert "_.col1" in serialized or "col1" in serialized - - -def test_serialize_ibis_method(): - fn = lambda t: t.amount.sum() # noqa: E731 - - result = expr_to_ibis_string(fn) - assert isinstance(result, Success | Failure) - - if isinstance(result, Success): - serialized = result.unwrap() - assert isinstance(serialized, str) - assert "sum()" in serialized - - -def test_deserialize_expr_string(): - expr_str = "_.amount * 2" - - deserialize_result = ibis_string_to_expr(expr_str) - assert isinstance(deserialize_result, Success | Failure) - - if isinstance(deserialize_result, Success): - restored_fn = deserialize_result.unwrap() - assert callable(restored_fn) - - -def test_round_trip_expression(): - fn = lambda t: t.price.mean() # noqa: E731 - - serialize_result = expr_to_ibis_string(fn) - if not isinstance(serialize_result, Success): - pytest.skip("Serialization failed") - - serialized = serialize_result.unwrap() - assert isinstance(serialized, str) - assert "mean()" in serialized - - def test_serialize_empty_dimensions(): result = serialize_dimensions({}) assert isinstance(result, Success) diff --git a/src/boring_semantic_layer/utils.py b/src/boring_semantic_layer/utils.py index 626bc09e..1e1ed7d9 100644 --- a/src/boring_semantic_layer/utils.py +++ b/src/boring_semantic_layer/utils.py @@ -2,14 +2,12 @@ import ast import importlib -import inspect import operator from collections.abc import Callable from pathlib import Path from typing import Any import yaml -from returns.maybe import Maybe, Nothing, Some from returns.result import Result, safe from toolz import curry @@ -141,13 +139,13 @@ def _check_callable_ref(module_name: str | None, qualname: str | None) -> None: ast.Dict, ast.keyword, ast.IfExp, - ast.Lambda, # Allow lambda expressions for ibis_string_to_expr + ast.Lambda, # Allow lambda expressions in YAML/agent-supplied strings ast.arguments, # Required for lambda function arguments ast.arg, # Required for individual lambda arguments } -# Helpers exposed by the Ibis modules in ``ibis_string_to_expr`` and agent +# Helpers exposed by the Ibis modules in agent # query contexts. In particular, backend/IO namespaces (duckdb, postgres, # read_*, connect, ...) are intentionally absent. SAFE_MODULE_CALLS = frozenset( @@ -466,175 +464,6 @@ def do_eval(): return do_eval() -def _extract_lambda_from_source(source: str) -> str: - if "lambda" not in source: - return source - - lambda_start = source.index("lambda") - lambda_expr = source[lambda_start:] - - for end_marker in [" #", " #", ",\n", "\n"]: - if end_marker in lambda_expr: - end_idx = lambda_expr.index(end_marker) - return lambda_expr[:end_idx].strip().rstrip(",") - - return lambda_expr.strip().rstrip(",") - - -def lambda_to_string(fn: Callable) -> Result[str, Exception]: - @safe - def do_extract(): - source_lines = inspect.getsourcelines(fn)[0] - source = "".join(source_lines).strip() - return _extract_lambda_from_source(source) - - return do_extract() - - -def _check_deferred(fn: Any) -> Maybe[str]: - from ibis.common.deferred import Deferred - - return Some(str(fn)) if isinstance(fn, Deferred) else Nothing - - -def _check_closure_vars(fn: Callable) -> Maybe[str]: - from ibis.common.deferred import Deferred - from returns.result import Success - - closure_vars = inspect.getclosurevars(fn) - - if not closure_vars.nonlocals: - return Nothing - - for name, value in closure_vars.nonlocals.items(): - if isinstance(value, Deferred): - return Some(str(value)) - if callable(value) and name == "expr": - result = expr_to_ibis_string(value) - if isinstance(result, Success): - return Some(result.unwrap()) - - return Nothing - - -@safe -def _try_ibis_introspection(fn: Callable) -> Maybe[str]: - from returns.result import Success - - from ._xorq import Deferred, _ - - result = fn(_) - if not isinstance(result, Deferred): - return Nothing - expr_str = str(result) - # Validate by attempting deserialization — if the string can't round-trip, - # it's useless (catches invalid syntax, internal function names like - # _finish_searched_case/ifelse that aren't in the eval context, etc.) - if not isinstance(ibis_string_to_expr(expr_str), Success): - return Nothing - return Some(expr_str) - - -def _extract_ibis_from_lambda_str(lambda_str: str) -> Maybe[str]: - if ":" not in lambda_str: - return Nothing - - body = lambda_str.split(":", 1)[1].strip() - param_part = lambda_str.split(":")[0] - param_names = param_part.replace("lambda", "").strip().split(",") - first_param = param_names[0].strip() - ibis_expr = body.replace(f"{first_param}.", "_.") - - return Some(ibis_expr) - - -def _try_source_extraction(fn: Callable) -> Maybe[str]: - from returns.result import Success - - lambda_str_result = lambda_to_string(fn) - return ( - _extract_ibis_from_lambda_str(lambda_str_result.unwrap()) - if isinstance(lambda_str_result, Success) - else Nothing - ) - - -def expr_to_ibis_string(fn: Callable) -> Result[str, Exception]: - @safe - def do_convert(): - if not callable(fn): - deferred_check = _check_deferred(fn) - if isinstance(deferred_check, Some): - return deferred_check.unwrap() - raise ValueError(f"Expected callable or Deferred, got {type(fn)}") - - checks = [ - lambda: _try_ibis_introspection(fn).value_or(Nothing), - lambda: _check_closure_vars(fn), - lambda: _try_source_extraction(fn), - ] - - for check in checks: - result = check() - if isinstance(result, Some): - return result.unwrap() - - return None - - return do_convert() - - -def ibis_string_to_expr(expr_str: str) -> Result[Callable, Exception]: - from returns.result import Failure, Success - - @safe - def do_convert(): - t_expr = expr_str.replace("_.", "t.") - lambda_str = f"lambda t: {t_expr}" - - import ibis - - def _build(flavor_ibis): - """Evaluate the lambda with ``ibis``/``_`` bound to one flavor.""" - eval_context = {"ibis": flavor_ibis, "_": flavor_ibis._} - allowed_names = {"ibis", "_", "t"} - try: - from ._xorq import api as xo - from ._xorq import ibis as xorq_ibis - - eval_context.update({"xorq_ibis": xorq_ibis, "xo": xo}) - allowed_names |= {"xorq_ibis", "xo"} - except ImportError: - pass - - result = safe_eval(lambda_str, context=eval_context, allowed_names=allowed_names) - if isinstance(result, Success): - return result.unwrap() - elif isinstance(result, Failure): - raise result.failure() - else: - raise ValueError(f"Unexpected result type: {type(result)}") - - # Eager evaluation validates the string up front; the returned wrapper - # re-binds ``ibis``/``_`` to the flavor (plain vs xorq-vendored) of the - # table it is called with, so eager constructors like ``ibis.literal`` - # compose with either flavor instead of silently mis-comparing. - fns = {id(ibis): _build(ibis)} - - def _flavored(t): - from .nested_compile import get_ibis_module - - flavor = get_ibis_module(t) - key = id(flavor) - if key not in fns: - fns[key] = _build(flavor) - return fns[key](t) - - return _flavored - - return do_convert() - - def _is_ibis_literal_node(value) -> bool: try: from ._xorq import Literal @@ -1155,8 +984,6 @@ def read_yaml_file(yaml_path: str | Path) -> dict: __all__ = [ "safe_eval", "SafeEvalError", - "expr_to_ibis_string", - "ibis_string_to_expr", "expr_to_structured", "structured_to_expr", "join_predicate_to_structured", From dfef20f9604015e92ae4411f5545d9afde1a76b0 Mon Sep 17 00:00:00 2001 From: Hussain Sultan Date: Tue, 18 Aug 2026 11:25:15 -0400 Subject: [PATCH 7/7] refactor: delete graph_utils' dead exports bfs, replace_nodes (real callers use the _xorq shim), to_node_safe, try_to_node, find_dimensions_and_measures, find_entity_dimensions, find_event_timestamp_dimensions, traverse_roots_with, extract_column_from_dimension and their private helpers had zero non-test callers. Their tests go with them; is_field/is_table_field stay (used by build_dependency_graph). graph_utils: 569 -> ~190 lines. Co-Authored-By: Claude Fable 5 --- src/boring_semantic_layer/graph_utils.py | 217 ++---------------- .../tests/test_graph_utils.py | 192 ---------------- 2 files changed, 16 insertions(+), 393 deletions(-) diff --git a/src/boring_semantic_layer/graph_utils.py b/src/boring_semantic_layer/graph_utils.py index 461bc5ba..c2cd6ec7 100644 --- a/src/boring_semantic_layer/graph_utils.py +++ b/src/boring_semantic_layer/graph_utils.py @@ -6,19 +6,12 @@ from ibis.expr.operations.core import Node as IbisNode from ibis.expr.types import Expr as IbisExpr -from returns.maybe import Maybe, Nothing, Some from returns.result import Result, Success, safe -from ._xorq import ( - Expr as XorqExpr, -) from ._xorq import ( Graph, Node, ) -from ._xorq import ( - replace_nodes as _xorq_replace_nodes, -) from ._xorq import ( to_node as _xorq_to_node, ) @@ -43,6 +36,22 @@ def _collect_field_types() -> tuple[type, ...]: FIELD_TYPES = _collect_field_types() +__all__ = [ + "gen_children_of", + "to_node", + "walk_nodes", + "Graph", + "Node", + "graph_predecessors", + "graph_successors", + "graph_bfs", + "graph_invert", + "graph_to_dict", + "build_dependency_graph", + "build_column_index_from_roots", +] + + def is_field(node: Any) -> bool: """Check if node is a Field from either ibis or xorq.""" return isinstance(node, FIELD_TYPES) @@ -62,31 +71,6 @@ def check(node: Any) -> bool: return check -__all__ = [ - "bfs", - "gen_children_of", - "replace_nodes", - "to_node", - "walk_nodes", - "to_node_safe", - "try_to_node", - "find_dimensions_and_measures", - "find_entity_dimensions", - "find_event_timestamp_dimensions", - "Graph", - "Node", - "graph_predecessors", - "graph_successors", - "graph_bfs", - "graph_invert", - "graph_to_dict", - "build_dependency_graph", - "extract_column_from_dimension", - "build_column_index_from_roots", - "traverse_roots_with", -] - - def to_node(maybe_expr: Any) -> Node: """Convert expression to node, handling various types.""" if isinstance(maybe_expr, IbisNode): @@ -102,33 +86,6 @@ def gen_children_of(node: Node) -> tuple[Node, ...]: return tuple(to_node(child) for child in children) -def bfs(expr) -> Graph: - """ - Build a graph using breadth-first search. - - This is fundamentally imperative - keep it simple and clear. - """ - from collections import deque - - start = to_node(expr) - queue = deque([start]) - graph_dict = {} - - while queue: - node = queue.popleft() - if node in graph_dict: - continue - - children = gen_children_of(node) - graph_dict[node] = children - - for child in children: - if child not in graph_dict: - queue.append(child) - - return Graph(graph_dict) - - def walk_nodes(node_types, expr): """ Walk nodes in depth-first order, yielding nodes of specified types. @@ -155,77 +112,6 @@ def walk_nodes(node_types, expr): stack.append(child) -def replace_nodes(replacer, expr): - """Replace nodes in an expression tree. - - xorq's ``replace_nodes`` only understands xorq's vendored-ibis nodes and - raises on plain-ibis ones. Since BSL coexists with both flavors, dispatch - plain-ibis nodes to ibis's native ``Node.replace`` (normalising the - ``None`` kwargs ibis passes for unchanged children to ``{}`` to match the - xorq replacer contract) and xorq nodes to xorq's ``replace_nodes``. - """ - node = to_node(expr) - if isinstance(node, IbisNode): - new_node = node.replace(lambda n, kwargs: replacer(n, kwargs if kwargs is not None else {})) - return new_node.to_expr() - return _xorq_replace_nodes(replacer, node).to_expr() - - -@safe(exceptions=(ValueError,)) -def to_node_safe(maybe_expr: Any) -> Node: - """ - Safely convert to node, returning Result. - - Public API that only catches ValueError since that's the expected - error type for invalid expression inputs from user code. - """ - return to_node(maybe_expr) - - -def try_to_node(child: Any) -> Maybe[Node]: - """Try to convert to node, returning Maybe.""" - return to_node_safe(child).map(Some).value_or(Nothing) - - -def find_dimensions_and_measures( - expr: IbisExpr | XorqExpr, -) -> tuple[dict[str, Any], dict[str, Any]]: - """ - Find dimensions and measures in expression. - - Uses functional composition for field extraction. - """ - from .ops import ( - _find_all_root_models, - _get_field_dict, - _merge_fields_with_prefixing, - ) - - roots = _find_all_root_models(to_node(expr)) - - dimensions = _merge_fields_with_prefixing(roots, lambda r: _get_field_dict(r, "dimensions")) - measures = _merge_fields_with_prefixing(roots, lambda r: _get_field_dict(r, "measures")) - - return (dimensions, measures) - - -def _filter_by_attribute(items: dict[str, Any], attr: str) -> dict[str, Any]: - """Filter dictionary items by attribute value.""" - return {name: item for name, item in items.items() if getattr(item, attr, False)} - - -def find_entity_dimensions(expr: IbisExpr | XorqExpr) -> dict[str, Any]: - """Find all entity dimensions in the expression tree.""" - dimensions, _ = find_dimensions_and_measures(expr) - return _filter_by_attribute(dimensions, "is_entity") - - -def find_event_timestamp_dimensions(expr: IbisExpr | XorqExpr) -> dict[str, Any]: - """Find all event timestamp dimensions in the expression tree.""" - dimensions, _ = find_dimensions_and_measures(expr) - return _filter_by_attribute(dimensions, "is_event_timestamp") - - def graph_predecessors(graph: dict[str, dict], node: str) -> set[str]: """Get direct dependencies of a node.""" return set(graph.get(node, {}).get("deps", {}).keys()) @@ -455,78 +341,7 @@ def classify_field(f): return {f.name: classify_field(f) for f in fields} -def traverse_roots_with( - roots: Sequence[Any], transform: Callable[[Any], Result[Any, Exception]] -) -> Result[list[Any], Exception]: - """ - Traverse semantic table roots and apply a transformation function to each. - - This is a generic traversal utility that handles errors safely using the - returns library. Short-circuits on first error. - - Args: - roots: Sequence of semantic table roots - transform: Function to apply to each root (root -> Result[T, Exception]) - - Returns: - Result containing list of successful transformations or first error - """ - - # Use railway-oriented programming with .bind for proper error propagation - def accumulate_result( - acc_result: Result[list[Any], Exception], root: Any - ) -> Result[list[Any], Exception]: - # Short-circuit if already failed - return acc_result.bind( - lambda acc_list: transform(root).map(lambda value: acc_list + [value]) - ) - - return functools_reduce(accumulate_result, roots, Success([])) - - -def extract_column_from_dimension(dimension: Any, table: Any) -> Maybe[str]: - """ - Extract the column name accessed by a dimension expression. - - Handles both Deferred expressions (_.column) and regular callables (lambda t: t.column). - Uses the returns library for safe extraction without exceptions. - - Args: - dimension: The dimension object or callable - table: The table to resolve against - - Returns: - Maybe[str] containing the column name if successful, Nothing otherwise - """ - from .ops import _extract_columns_from_callable, _is_deferred - - expr = dimension.expr if hasattr(dimension, "expr") else dimension - - if _is_deferred(expr): - return _safe_extract_from_deferred(expr, table) - - if callable(expr): - extraction_result = _extract_columns_from_callable(expr, table) - if extraction_result.is_success() and extraction_result.columns: - return Some(next(iter(extraction_result.columns))) - - return Nothing - - @safe -def _extract_from_deferred(deferred_expr: Any, table: Any) -> str: - resolved = deferred_expr.resolve(table) - if hasattr(resolved, "get_name"): - return resolved.get_name() - raise ValueError("No get_name method") - - -def _safe_extract_from_deferred(deferred_expr: Any, table: Any) -> Maybe[str]: - """Safely extract column name from deferred expression.""" - result = _extract_from_deferred(deferred_expr, table) - return result.map(Some).value_or(Nothing) - - def build_column_index_from_roots( roots: Sequence[Any], ) -> Result[dict[str, list[int]], Exception]: diff --git a/src/boring_semantic_layer/tests/test_graph_utils.py b/src/boring_semantic_layer/tests/test_graph_utils.py index 8bb66f97..395df570 100644 --- a/src/boring_semantic_layer/tests/test_graph_utils.py +++ b/src/boring_semantic_layer/tests/test_graph_utils.py @@ -2,36 +2,10 @@ import pytest from ibis.expr.operations.relations import Aggregate -from boring_semantic_layer.expr import SemanticModel from boring_semantic_layer.graph_utils import ( - bfs, - find_dimensions_and_measures, - find_entity_dimensions, - find_event_timestamp_dimensions, - gen_children_of, - replace_nodes, to_node, walk_nodes, ) -from boring_semantic_layer.ops import Dimension, Measure - - -def test_bfs_and_gen_children_of_simple_expr(): - # Build a simple aggregation expression - t = ibis.memtable({"x": [1, 2, 3]}) - expr = t.group_by("x").aggregate(sum_x=t.x.sum()) - - # BFS should map each Node to its children - graph = bfs(expr) - root = to_node(expr) - assert root in graph, "Root node not in BFS graph" - children = graph[root] - assert isinstance(children, tuple) and children, "Expected non-empty children tuple" - - # gen_children_of should agree for the root - direct = gen_children_of(root) - assert isinstance(direct, tuple) - assert set(direct) == set(children) def test_walk_nodes_finds_aggregation(): @@ -46,169 +20,3 @@ def test_walk_nodes_finds_aggregation(): def test_to_node_errors_on_bad_input(): with pytest.raises(ValueError): to_node(123) - - -def test_replace_nodes_identity_replacer_leaves_expr_unchanged(): - expr = ibis.literal(1) + ibis.literal(2) - # A replacer that always returns the original op should leave the expression unchanged - new_expr = replace_nodes(lambda op, kwargs: op, expr) - assert str(new_expr) == str(expr) - - -def test_find_dimensions_and_measures_no_semantic_table(): - t = ibis.memtable({"x": [1, 2, 3]}) - dims, meas = find_dimensions_and_measures(t) - assert dims == {} - assert meas == {} - - -def test_find_dimensions_and_measures_semantic_table(): - t = ibis.memtable({"x": [1, 2, 3]}) - dims_defs = {"x": Dimension(expr=lambda tbl: tbl.x, description="dim x")} - meas_defs = { - "sum_x": Measure(expr=lambda tbl: tbl.x.sum(), description="measure sum_x"), - } - semantic = SemanticModel( - table=t, - dimensions=dims_defs, - measures=meas_defs, - calc_measures=None, - name="mytable", - ) - # SemanticModel is the Expression - use it directly - dims, meas = find_dimensions_and_measures(semantic) - assert dims == {"mytable.x": dims_defs["x"]} - assert meas == {"mytable.sum_x": meas_defs["sum_x"]} - - -def test_find_entity_dimensions_no_semantic_table(): - """Test that find_entity_dimensions returns empty dict for non-semantic tables.""" - t = ibis.memtable({"x": [1, 2, 3]}) - entities = find_entity_dimensions(t) - assert entities == {} - - -def test_find_entity_dimensions_no_entities(): - """Test that find_entity_dimensions returns empty dict when no entity dimensions exist.""" - t = ibis.memtable({"x": [1, 2, 3]}) - dims_defs = {"x": Dimension(expr=lambda tbl: tbl.x, description="regular dim")} - semantic = SemanticModel( - table=t, - dimensions=dims_defs, - measures=None, - calc_measures=None, - name="mytable", - ) - entities = find_entity_dimensions(semantic) - assert entities == {} - - -def test_find_entity_dimensions_with_entities(): - """Test that find_entity_dimensions finds entity dimensions correctly.""" - t = ibis.memtable({"business_id": [1, 2, 3], "user_id": [10, 20, 30], "x": [100, 200, 300]}) - dims_defs = { - "business_id": Dimension(expr=lambda tbl: tbl.business_id, is_entity=True), - "user_id": Dimension(expr=lambda tbl: tbl.user_id, is_entity=True), - "x": Dimension(expr=lambda tbl: tbl.x, description="regular dim"), - } - semantic = SemanticModel( - table=t, - dimensions=dims_defs, - measures=None, - calc_measures=None, - name="features", - ) - entities = find_entity_dimensions(semantic) - assert len(entities) == 2 - assert "features.business_id" in entities - assert "features.user_id" in entities - assert entities["features.business_id"].is_entity is True - assert entities["features.user_id"].is_entity is True - - -def test_find_event_timestamp_dimensions_no_semantic_table(): - """Test that find_event_timestamp_dimensions returns empty dict for non-semantic tables.""" - t = ibis.memtable({"x": [1, 2, 3]}) - timestamps = find_event_timestamp_dimensions(t) - assert timestamps == {} - - -def test_find_event_timestamp_dimensions_no_timestamps(): - """Test that find_event_timestamp_dimensions returns empty dict when no event timestamps exist.""" - t = ibis.memtable({"x": [1, 2, 3]}) - dims_defs = {"x": Dimension(expr=lambda tbl: tbl.x, description="regular dim")} - semantic = SemanticModel( - table=t, - dimensions=dims_defs, - measures=None, - calc_measures=None, - name="mytable", - ) - timestamps = find_event_timestamp_dimensions(semantic) - assert timestamps == {} - - -def test_find_event_timestamp_dimensions_with_timestamp(): - """Test that find_event_timestamp_dimensions finds event timestamp dimensions correctly.""" - t = ibis.memtable( - { - "statement_date": ["2024-01-01", "2024-01-02"], - "order_date": ["2024-01-01", "2024-01-02"], - "x": [100, 200], - } - ) - dims_defs = { - "statement_date": Dimension( - expr=lambda tbl: tbl.statement_date, - is_event_timestamp=True, - is_time_dimension=True, - smallest_time_grain="TIME_GRAIN_DAY", - ), - "order_date": Dimension(expr=lambda tbl: tbl.order_date, is_time_dimension=True), - "x": Dimension(expr=lambda tbl: tbl.x, description="regular dim"), - } - semantic = SemanticModel( - table=t, - dimensions=dims_defs, - measures=None, - calc_measures=None, - name="features", - ) - timestamps = find_event_timestamp_dimensions(semantic) - assert len(timestamps) == 1 - assert "features.statement_date" in timestamps - assert timestamps["features.statement_date"].is_event_timestamp is True - - -def test_find_entity_and_event_timestamp_together(): - """Test that both entity and event timestamp dimensions can be found in the same model.""" - t = ibis.memtable( - { - "business_id": [1, 2, 3], - "statement_date": ["2024-01-01", "2024-01-02", "2024-01-03"], - "balance": [1000, 2000, 3000], - } - ) - dims_defs = { - "business_id": Dimension(expr=lambda tbl: tbl.business_id, is_entity=True), - "statement_date": Dimension( - expr=lambda tbl: tbl.statement_date, - is_event_timestamp=True, - ), - } - semantic = SemanticModel( - table=t, - dimensions=dims_defs, - measures=None, - calc_measures=None, - name="balance", - ) - - entities = find_entity_dimensions(semantic) - timestamps = find_event_timestamp_dimensions(semantic) - - assert len(entities) == 1 - assert "balance.business_id" in entities - - assert len(timestamps) == 1 - assert "balance.statement_date" in timestamps