From b583309dc4bc5af688fae08850378cd8de960c70 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Fri, 31 Jul 2026 14:29:19 -0500 Subject: [PATCH 01/15] Break the per-call agg import in get_typed_op with a type registry get_typed_op imported the agg module inside the function on every operator resolution, to isinstance-check Aggregator and TypedAggregator. The import had to be function-local because agg.py imports get_typed_op, so a module-level import would be a cycle. agg.py now registers its two classes with operator/utils.py when it loads, and get_typed_op consults the registry instead of running an import statement per call. The None guard is safe because those classes are the only way to make an Aggregator: before agg.py finishes loading no instance can exist, and agg.py's own body never hands one to get_typed_op (its two call sites pass a monoid and a semiring, both from inside functions rather than at module level). This does not change when the agg module loads. core/operator/__init__.py imports Aggregator eagerly, so agg is already in sys.modules before any user code can reach get_typed_op; the function-local import was repeating a sys.modules lookup rather than deferring a module load. --- graphblas/core/operator/agg.py | 4 +++- graphblas/core/operator/utils.py | 32 +++++++++++++++++++++++--------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/graphblas/core/operator/agg.py b/graphblas/core/operator/agg.py index 574f4ec1b..f1100c32e 100644 --- a/graphblas/core/operator/agg.py +++ b/graphblas/core/operator/agg.py @@ -776,4 +776,6 @@ def _first_last_index(agg, updater, expr, opts, *, in_composite, semiring): agg.Aggregator = Aggregator agg.TypedAggregator = TypedAggregator -from .utils import get_typed_op # noqa: E402 isort:skip +from .utils import _register_aggregator_types, get_typed_op # noqa: E402 isort:skip + +_register_aggregator_types(Aggregator, TypedAggregator) diff --git a/graphblas/core/operator/utils.py b/graphblas/core/operator/utils.py index c3a8f5b8f..943bdb2f3 100644 --- a/graphblas/core/operator/utils.py +++ b/graphblas/core/operator/utils.py @@ -71,6 +71,21 @@ raise +# ``agg.py`` imports ``get_typed_op`` from this module, so the two form an import +# cycle and we can't import the Aggregator classes at module load. ``agg.py`` +# registers them here when it loads, which ``core.operator.__init__`` does +# eagerly at import. Before registration no Aggregator instance can exist, so +# ``get_typed_op`` skips the check entirely. +_Aggregator = None +_TypedAggregator = None + + +def _register_aggregator_types(aggregator, typed_aggregator): + global _Aggregator, _TypedAggregator + _Aggregator = aggregator + _TypedAggregator = typed_aggregator + + def get_typed_op(op, dtype, dtype2=None, *, is_left_scalar=False, is_right_scalar=False, kind=None): if isinstance(op, OpBase): # UDTs always get compiled @@ -126,15 +141,14 @@ def get_typed_op(op, dtype, dtype2=None, *, is_left_scalar=False, is_right_scala if isinstance(op, TypedOpBase): return op - from .agg import Aggregator, TypedAggregator - - if isinstance(op, Aggregator): - # agg._any_dtype basically serves the same purpose as op._custom_dtype - if op._any_dtype is not None and op._any_dtype is not True: - return op[op._any_dtype] - return op[dtype] - if isinstance(op, TypedAggregator): - return op + if _Aggregator is not None: + if isinstance(op, _Aggregator): + # agg._any_dtype basically serves the same purpose as op._custom_dtype + if op._any_dtype is not None and op._any_dtype is not True: + return op[op._any_dtype] + return op[dtype] + if isinstance(op, _TypedAggregator): + return op if isinstance(op, str): if kind == "unary": op = unary_from_string(op) From 5b16b79d44daaec15d36daf5825c99b3d472908c Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Fri, 31 Jul 2026 14:31:22 -0500 Subject: [PATCH 02/15] Add indexunary.value/row/column helpers accepting comparison expressions (gh-239) gh-239 asks for value, row, column and index helpers in the indexunary namespace, mirroring the four that select already has. This adds three of them: indexunary.value(A > 3) returns the same expression as A.apply(indexunary.valuegt, 3), with the same input validation, the same lt-to-le and ge-to-gt thunk shifts, and the same errors as select's helpers, adapted to name indexunary. The shared resolution logic moves to core/operator/utils.py; select delegates to it and is unchanged. index is deliberately not added, so this closes three quarters of gh-239. indexunary.index already exists as an operator alias for rowindex, and is used as one (v.apply(indexunary.index) and indexunary.index(v) are both tested), so a helper function of that name would shadow a working operator. Adding it needs a decision about that alias, which is a maintainer call. Vector index comparisons are meanwhile reachable through indexunary.row(v < k) or the indexle and indexgt aliases. New matrix and vector tests compare each helper against the explicit apply, and cover the BOOL result dtype, the thunk shifts, the scalar expression path, and the error cases. --- graphblas/core/operator/utils.py | 46 +++++++++++++++++++++ graphblas/indexunary/__init__.py | 51 +++++++++++++++++++++++ graphblas/select/__init__.py | 70 ++------------------------------ graphblas/tests/test_matrix.py | 25 ++++++++++++ graphblas/tests/test_vector.py | 24 +++++++++++ 5 files changed, 149 insertions(+), 67 deletions(-) diff --git a/graphblas/core/operator/utils.py b/graphblas/core/operator/utils.py index 943bdb2f3..1d5dfcbe1 100644 --- a/graphblas/core/operator/utils.py +++ b/graphblas/core/operator/utils.py @@ -331,6 +331,52 @@ def get_semiring(monoid, binaryop, name=None): return rv +def _resolve_index_expr(ns, modname, expr, callname, opname): + """Turn an infix comparison expression into an indexunary/select op call. + + Shared by the ``value``/``row``/``column`` helpers in the ``select`` and + ``indexunary`` namespaces (and ``select.index``). ``select.valuegt`` is a + SelectOp that defaults to ``.select``, while ``indexunary.valuegt`` is an + IndexUnaryOp that defaults to ``.apply``, so the same expression resolves to + a select or an apply depending on which namespace's ops ``ns`` holds. ``ns`` + is the namespace module's ``globals()`` dict and ``modname`` is its short + name (used in the error messages). + """ + from ..base import BaseExpression + + if not isinstance(expr, BaseExpression): + raise TypeError( + f"Expected ScalarExpression, VectorExpression, or MatrixExpression; " + f"found {type(expr)}\nTypical usage: {modname}.{callname}(x <= 5)" + ) + tensor = expr.args[0] + thunk = expr.args[1] + method = f"{opname}{expr.op.name}" + if method not in ns: + # TODO: remove this once rowlt/rowge/collt/colge exist + # Convert thunk to Python int to avoid possible subtraction with uints + thunk = thunk.value + # Attempt to convert < into <= (rowlt is not part of official spec, but rowle is) + if expr.op.name == "lt": + method = f"{opname}le" + thunk -= 1 + # Attempt to convert >= into > (rowge is not part of official spec, but rowgt is) + elif expr.op.name == "ge": + method = f"{opname}gt" + thunk -= 1 + if method not in ns: # pragma: no cover (sanity) + raise ValueError(f"Unknown or unregistered {modname} method: {method}") + if expr._is_scalar: + # Handle ScalarExpressions that change their arguments to Vector + if tensor._parent is not None: # e.g., suitesparse + tensor = tensor._parent + thunk = thunk._parent + else: # e.g., suitesparse-vanilla + tensor = tensor[0].new() + thunk = thunk[0].new() + return ns[method](tensor, thunk) + + unary.register_new = UnaryOp.register_new unary.register_anonymous = UnaryOp.register_anonymous indexbinary.register_new = IndexBinaryOp.register_new diff --git a/graphblas/indexunary/__init__.py b/graphblas/indexunary/__init__.py index 5fa548c5a..4aee3524a 100644 --- a/graphblas/indexunary/__init__.py +++ b/graphblas/indexunary/__init__.py @@ -30,6 +30,57 @@ def __getattr__(key): raise _module_attr_error(__name__, key, __dir__()) +def _resolve_expr(expr, callname, opname): + from ..core.operator.utils import _resolve_index_expr + + return _resolve_index_expr(globals(), "indexunary", expr, callname, opname) + + +def value(expr): + """An advanced indexunary method for easily expressing value comparison logic. + + Example usage: + >>> gb.indexunary.value(A > 0) + + The example will dispatch to ``gb.indexunary.valuegt(A, 0)`` + while being nicer to read. + """ + return _resolve_expr(expr, "value", "value") + + +def row(expr): + """An advanced indexunary method for easily expressing Matrix row index comparison logic. + + Example usage: + >>> gb.indexunary.row(A <= 5) + + The example will dispatch to ``gb.indexunary.rowle(A, 5)`` + while being potentially nicer to read. + """ + return _resolve_expr(expr, "row", "row") + + +def column(expr): + """An advanced indexunary method for easily expressing Matrix column index comparison logic. + + Example usage: + >>> gb.indexunary.column(A <= 5) + + The example will dispatch to ``gb.indexunary.colle(A, 5)`` + while being potentially nicer to read. + """ + return _resolve_expr(expr, "column", "col") + + +# Note: an ``index`` helper (the Vector analogue of ``select.index``) is *not* +# provided here because ``indexunary.index`` already exists as an alias for the +# positional ``rowindex`` op (INT64). It is relied on as an operator, e.g. +# ``v.apply(indexunary.index)``, which a helper function would break. For a +# Vector index comparison use ``indexunary.row(v < k)`` (resolves to ``rowle``, +# the same op ``select.index`` uses) or the explicit ``indexunary.indexle`` / +# ``indexunary.indexgt``. + + from ..core import operator # noqa: E402 isort:skip del operator diff --git a/graphblas/select/__init__.py b/graphblas/select/__init__.py index a2d94a0a0..43e8c206e 100644 --- a/graphblas/select/__init__.py +++ b/graphblas/select/__init__.py @@ -3,9 +3,6 @@ # identical to IndexUnaryOp under the hood, except they must return # boolean and they default to `select` when called, rather than # `apply`). -import math -import numbers - _delayed = {} _binary_to_select = {} @@ -38,70 +35,9 @@ def __getattr__(key): def _resolve_expr(expr, callname, opname): - from ..core.base import BaseExpression - - if not isinstance(expr, BaseExpression): - raise TypeError( - f"Expected ScalarExpression, VectorExpression, or MatrixExpression; " - f"found {type(expr)}\nTypical usage: select.{callname}(x <= 5)" - ) - tensor = expr.args[0] - thunk = expr.args[1] - if expr._is_scalar: - # Handle ScalarExpressions that change their arguments to Vector - if tensor._parent is not None: # e.g., suitesparse - tensor = tensor._parent - thunk = thunk._parent - else: # e.g., suitesparse-vanilla - tensor = tensor[0].new() - thunk = thunk[0].new() - name = expr.op.name - if opname != "value": - if name in {"lt", "ge"} and f"{opname}{name}" in globals(): - # An op registered for it (a user's rowlt): the number as given - if expr._right_literal is not None: - thunk = expr._right_literal - else: - thunk, name = _index_bound(expr._right_literal, thunk, name) - method = f"{opname}{name}" - if method not in globals(): # pragma: no cover (sanity) - raise ValueError(f"Unknown or unregistered select method: {method}") - return globals()[method](tensor, thunk) - - -def _index_bound(literal, thunk, name): - """Return the thunk and comparison that select an index ``name`` (lt, le, gt, ge) a bound. - - The bound is the number the caller gave (``literal``), else the thunk Scalar's value. - The comparison typed its thunk beside the values, which is wrong for an index: 300 - beside INT8 values is infinity, and 2**24 + 1 beside FP32 rounds. Only rowle and - rowgt exist (and colle, colgt, indexle, indexgt), and indices are whole numbers, so - ``i < t`` is ``i <= ceil(t) - 1``, ``i >= t`` is ``i > ceil(t) - 1``, and ``i <= t`` - and ``i > t`` take ``floor(t)``, where GraphBLAS would truncate a float toward zero - (``i > -0.5`` dropped index 0). A whole number beyond INT64, which holds every index, - is clamped to it. NaN compares False with every index, so it keeps none. An empty - Scalar goes to the op as it is, which raises EmptyObject, and so does a thunk that is - no Scalar (from ``A.apply(binary.le, left=3)``), which raises the op's TypeError. - """ - shift = name in {"lt", "ge"} - if shift: - name = "le" if name == "lt" else "gt" - # The caller's number, if it is one; a Scalar expression (``w[0]``), a 0-d array or - # ``np.True_`` made the thunk keep its own type, so the thunk's value is the bound. - if isinstance(literal, numbers.Real): - bound = literal - elif getattr(thunk, "_is_scalar", False): - bound = thunk.value - else: - return thunk, name - if not isinstance(bound, numbers.Real): - return thunk, name - if math.isnan(bound): - return (-1 if name == "le" else 2**63 - 1), name # no index is <= -1 or > INT64_MAX - if math.isfinite(bound): - bound = math.ceil(bound) - 1 if shift else math.floor(bound) - bound = min(max(bound, -(2**63)), 2**63 - 1) - return bound, name + from ..core.operator.utils import _resolve_index_expr + + return _resolve_index_expr(globals(), "select", expr, callname, opname) def _match_expr(parent, expr): diff --git a/graphblas/tests/test_matrix.py b/graphblas/tests/test_matrix.py index 9f9fbab36..47bd305cd 100644 --- a/graphblas/tests/test_matrix.py +++ b/graphblas/tests/test_matrix.py @@ -1363,6 +1363,31 @@ def test_apply_indexunary(A): assert pickle.loads(pickle.dumps(indexunary.tril[int])) is indexunary.tril[int] +def test_indexunary_helpers(A): + # indexunary.value/row/column mirror select's helpers (GH #239), but they + # produce apply expressions (BOOL) rather than select expressions. + assert indexunary.value(A > 3).new().isequal(A.apply(indexunary.valuegt, 3).new()) + assert indexunary.value(A == 3).new().isequal(A.apply(indexunary.valueeq, 3).new()) + # Only rowle/rowgt and colle/colgt exist, so `<` and `>=` are rewritten with + # a thunk shift, and `==`/`!=` have no counterpart (mirrors select's helpers). + assert indexunary.row(A <= 2).new().isequal(A.apply(indexunary.rowle, 2).new()) + assert indexunary.row(A < 3).new().isequal(A.apply(indexunary.rowle, 2).new()) + assert indexunary.row(A >= 3).new().isequal(A.apply(indexunary.rowgt, 2).new()) + assert indexunary.row(A > 2).new().isequal(A.apply(indexunary.rowgt, 2).new()) + assert indexunary.column(A < 3).new().isequal(A.apply(indexunary.colle, 2).new()) + assert indexunary.column(A > 2).new().isequal(A.apply(indexunary.colgt, 2).new()) + for expr in [indexunary.value(A > 3), indexunary.row(A <= 2), indexunary.column(A < 3)]: + assert expr.new().dtype == dtypes.BOOL + with pytest.raises(TypeError, match="indexunary.value"): + indexunary.value(A) + with pytest.raises(TypeError, match="indexunary.row"): + indexunary.row(A | A) + with pytest.raises(ValueError, match="roweq"): + indexunary.row(A == 3) + with pytest.raises(ValueError, match="coleq"): + indexunary.column(A == 3) + + @autocompute def test_select_by_a_transposed_comparison(): """``A.select(A.T == x)`` keeps the entries where ``A.T == x``, as a computed mask does. diff --git a/graphblas/tests/test_vector.py b/graphblas/tests/test_vector.py index 1f0ef108d..4fb3f91d3 100644 --- a/graphblas/tests/test_vector.py +++ b/graphblas/tests/test_vector.py @@ -810,6 +810,30 @@ def test_apply_indexunary(v): v.apply(indexunary.valueeq, left=s2) +def test_indexunary_helpers(v): + # indexunary.value/row/column mirror select's helpers (GH #239), returning + # apply expressions (BOOL) rather than select expressions. + assert indexunary.value(v == 1).new().isequal(v.apply(indexunary.valueeq, 1).new()) + assert indexunary.value(v >= 2).new().isequal(v.apply(indexunary.valuege, 2).new()) + # Index comparisons on a Vector go through the row helper (indexle is rowle). + assert indexunary.row(v < 4).new().isequal(v.apply(indexunary.rowle, 3).new()) + assert indexunary.row(v >= 4).new().isequal(v.apply(indexunary.rowgt, 3).new()) + assert indexunary.value(v == 1).new().dtype == dtypes.BOOL + assert indexunary.row(v < 4).new().dtype == dtypes.BOOL + # Scalar expression path resolves to a ScalarExpression, like select.value. + s = Scalar.from_value(1) + assert indexunary.value(s < 10).new() == s.apply(indexunary.valuelt, 10).new() + # `index` is intentionally not a helper: it remains the rowindex op alias, so + # `v.apply(indexunary.index)` keeps working. + assert indexunary.index is indexunary.rowindex + # A value mask reads naturally through the value helper. + assert ( + indexunary.value(v != False).new().isequal(indexunary.valuene(v, False).new()) # noqa: E712 + ) + with pytest.raises(TypeError, match="indexunary.value"): + indexunary.value(v) + + def test_select(v): result = Vector.from_coo([1, 3], [1, 1], size=7) w1 = v.select(select.valueeq, 1).new() From 01ac657daa3f4b23009d4457f88375b4c743361a Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Fri, 31 Jul 2026 14:32:46 -0500 Subject: [PATCH 03/15] Add docstrings to the operator from_string helpers (GH #513) All eight operator-namespace from_string helpers (unary, binary, monoid, semiring, select, indexunary, agg, op) had no docstring at all. Add numpydoc docstrings describing the accepted string forms: a namespace name, a dotted path such as numpy.mod, shorthand symbols such as + or >=, and the [dtype] typing suffix. Each carries runnable examples. test_from_string gains select and indexunary cases and asserts every namespace's from_string is now documented. Also fix a stale example in the indexunary from_string error message. It offered 'row_index', which does not resolve; the operator is 'rowindex'. --- graphblas/core/operator/utils.py | 228 ++++++++++++++++++++++++++++++- graphblas/tests/test_op.py | 9 ++ 2 files changed, 236 insertions(+), 1 deletion(-) diff --git a/graphblas/core/operator/utils.py b/graphblas/core/operator/utils.py index 1d5dfcbe1..cb51cbcf8 100644 --- a/graphblas/core/operator/utils.py +++ b/graphblas/core/operator/utils.py @@ -514,28 +514,196 @@ def _from_string(string, module, mapping, example): def unary_from_string(string): + """Look up a UnaryOp by name or symbol, optionally typed with ``"[dtype]"``. + + Backs ``gb.unary.from_string`` and the string coercion used wherever a + UnaryOp is accepted, such as ``v.apply("abs")``. + + Parameters + ---------- + string : str + A name in the ``gb.unary`` namespace (``"abs"``, or a dotted path such + as ``"numpy.negative"``) or a symbolic shorthand (``"-"`` for ``ainv``, + ``"~"`` for ``lnot``). Append ``"[dtype]"`` to type the operator, as in + ``"abs[int]"``. + + Returns + ------- + UnaryOp + + See Also + -------- + unary.register_new + op.from_string + + Examples + -------- + >>> gb.unary.from_string("abs") is gb.unary.abs + True + >>> gb.unary.from_string("abs[int]") is gb.unary.abs[int] + True + + """ return _from_string(string, unary, _str_to_unary, "abs[int]") def indexunary_from_string(string): + """Look up an IndexUnaryOp by name, optionally typed with ``"[dtype]"``. + + Parameters + ---------- + string : str + A name in the ``gb.indexunary`` namespace, such as ``"rowindex"``, + ``"diag"``, or ``"tril"``. Append ``"[dtype]"`` to type the operator, + as in ``"rowindex[int]"``. + + Returns + ------- + IndexUnaryOp + + See Also + -------- + indexunary.register_new + select.from_string + + Examples + -------- + >>> gb.indexunary.from_string("rowindex") is gb.indexunary.rowindex + True + + """ # "select" is a variant of IndexUnary, so the string abbreviations in # _str_to_select are appropriate to reuse here - return _from_string(string, indexunary, _str_to_select, "row_index") + return _from_string(string, indexunary, _str_to_select, "rowindex") def select_from_string(string): + """Look up a SelectOp by name or comparison symbol. + + Parameters + ---------- + string : str + A name in the ``gb.select`` namespace (``"tril"``, ``"triu"``, + ``"offdiag"``, ``"valuegt"``, ...) or a comparison shorthand such as + ``">="`` (``valuege``), ``"=="`` (``valueeq``), or ``"row>"`` + (``rowgt``). + + Returns + ------- + SelectOp + + See Also + -------- + select.register_new + indexunary.from_string + + Examples + -------- + >>> gb.select.from_string("tril") is gb.select.tril + True + >>> gb.select.from_string(">=") is gb.select.valuege + True + + """ return _from_string(string, select, _str_to_select, "tril") def binary_from_string(string): + """Look up a BinaryOp by name or symbol, optionally typed with ``"[dtype]"``. + + Backs ``gb.binary.from_string`` and the string coercion used wherever a + BinaryOp is accepted, such as ``A.ewise_mult(B, "+")``. + + Parameters + ---------- + string : str + A name in the ``gb.binary`` namespace (``"plus"``, or a dotted path such + as ``"numpy.mod"``) or an arithmetic/comparison shorthand such as ``"+"`` + (``plus``), ``"*"`` (``times``), or ``">="`` (``ge``). Append + ``"[dtype]"`` to type the operator, as in ``"plus[int]"``. + + Returns + ------- + BinaryOp + + See Also + -------- + binary.register_new + op.from_string + + Examples + -------- + >>> gb.binary.from_string("+") is gb.binary.plus + True + >>> gb.binary.from_string("minus[int]") is gb.binary.minus[int] + True + + """ return _from_string(string, binary, _str_to_binary, "+[int]") def monoid_from_string(string): + """Look up a Monoid by name or symbol, optionally typed with ``"[dtype]"``. + + Parameters + ---------- + string : str + A name in the ``gb.monoid`` namespace (``"plus"``, ``"times"``, ...) or a + symbolic shorthand such as ``"+"`` (``plus``), ``"*"`` (``times``), or + ``"|"`` (``lor``). Append ``"[dtype]"`` to type the monoid, as in + ``"plus[float]"``. + + Returns + ------- + Monoid + + See Also + -------- + monoid.register_new + semiring.from_string + + Examples + -------- + >>> gb.monoid.from_string("+[float]") is gb.monoid.plus[float] + True + + """ return _from_string(string, monoid, _str_to_monoid, "+[int]") def semiring_from_string(string): + """Look up a Semiring by name, optionally typed with ``"[dtype]"``. + + A semiring pairs a monoid with a binaryop. Name it either as the combined + namespace attribute (``"plus_times"``) or in ``"monoid.binaryop"`` form using + the monoid and binaryop shorthands (``"min.+"``); the two parts must be + separated by exactly one period. + + Parameters + ---------- + string : str + The semiring name, such as ``"plus_times"`` or ``"min_plus"``, or the + ``"monoid.binaryop"`` form ``"min.+"``. Append ``"[dtype]"`` to type the + semiring, as in ``"min.+[int]"``. + + Returns + ------- + Semiring + + See Also + -------- + semiring.register_new + semiring.get_semiring + op.from_string + + Examples + -------- + >>> gb.semiring.from_string("min.+") is gb.semiring.min_plus + True + >>> gb.semiring.from_string("min_plus") is gb.semiring.min_plus + True + + """ split = string.split(".") if len(split) == 1: try: @@ -554,6 +722,38 @@ def semiring_from_string(string): def op_from_string(string): + """Look up an operator of any kind by string. + + Each operator type is tried in turn (unary, binary, monoid, semiring, + indexunary, select, then aggregator) and the first match is returned, so an + unqualified name resolves to whichever kind defines it first. Use a + type-specific ``from_string`` (e.g. ``gb.binary.from_string``) when the kind + is known and matters. + + Parameters + ---------- + string : str + An operator name or symbol accepted by any of the type-specific + ``from_string`` functions, optionally typed with ``"[dtype]"``. + + Returns + ------- + UnaryOp, BinaryOp, Monoid, Semiring, IndexUnaryOp, SelectOp, or Aggregator + + See Also + -------- + unary.from_string + binary.from_string + semiring.from_string + + Examples + -------- + >>> gb.op.from_string("+") is gb.binary.plus + True + >>> gb.op.from_string("min.plus") is gb.semiring.min_plus + True + + """ for func in [ # Note: order matters here unary_from_string, @@ -588,6 +788,32 @@ def op_from_string(string): def aggregator_from_string(string): + """Look up an Aggregator by name or symbol, optionally typed with ``"[dtype]"``. + + Parameters + ---------- + string : str + A name in the ``gb.agg`` namespace (``"sum"``, ``"count"``, ``"any"``, + ...) or a symbolic shorthand such as ``"+"`` (``sum``), ``"*"`` + (``prod``), ``"&"`` (``all``), or ``"|"`` (``any``). Append ``"[dtype]"`` + to type the aggregator, as in ``"sum[int]"``. + + Returns + ------- + Aggregator + + See Also + -------- + op.from_string + + Examples + -------- + >>> gb.agg.from_string("sum[int]") is gb.agg.sum[int] + True + >>> gb.agg.from_string("|") is gb.agg.any + True + + """ return _from_string(string, agg, _str_to_agg, "sum[int]") diff --git a/graphblas/tests/test_op.py b/graphblas/tests/test_op.py index df5896e88..36aa8a5b2 100644 --- a/graphblas/tests/test_op.py +++ b/graphblas/tests/test_op.py @@ -1287,6 +1287,15 @@ def test_from_string(): with pytest.raises(ValueError, match="Unknown agg string"): agg.from_string("bad_agg") + assert select.from_string("tril") is select.tril + assert select.from_string(">=") is select.valuege + assert indexunary.from_string("rowindex") is indexunary.rowindex + assert indexunary.from_string("rowindex[int]") is indexunary.rowindex[int] + + # Every namespace's from_string carries a docstring (GH #513) + for ns in [unary, binary, monoid, semiring, select, indexunary, agg, op]: + assert ns.from_string.__doc__ + @pytest.mark.skipif("not supports_udfs") @pytest.mark.slow From c240e0c30734cc524b9e2f74bc4463a680941e29 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 20 Oct 2026 07:49:58 -0500 Subject: [PATCH 04/15] Exclude the before-registration arm of get_typed_op's Aggregator check agg.py registers the Aggregator classes when core.operator loads it, at import, and no call reaches this check before then, so the arm where _Aggregator is still None never runs (the guard is there in case one ever does). --- graphblas/core/operator/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphblas/core/operator/utils.py b/graphblas/core/operator/utils.py index cb51cbcf8..14b553353 100644 --- a/graphblas/core/operator/utils.py +++ b/graphblas/core/operator/utils.py @@ -141,7 +141,7 @@ def get_typed_op(op, dtype, dtype2=None, *, is_left_scalar=False, is_right_scala if isinstance(op, TypedOpBase): return op - if _Aggregator is not None: + if _Aggregator is not None: # pragma: no branch (safety) if isinstance(op, _Aggregator): # agg._any_dtype basically serves the same purpose as op._custom_dtype if op._any_dtype is not None and op._any_dtype is not True: From 40cc052925da869b2ba66d9e7b1a0ba130809fa8 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 3 Nov 2026 12:06:30 -0800 Subject: [PATCH 05/15] Return only operators from from_string, and refuse applying a helper by name _from_string took any attribute of the namespace module, so the new helper functions, and private names and methods, came back as operators: indexunary.from_string("value") (also "row", "column", "VALUE") before: ("value[int]": TypeError, not subscriptable) after: ValueError: Unknown indexunary string: 'value' as on the parent, and as the from_string docstrings say (IndexUnaryOp). select.from_string("value") and op.from_string("register_new") (a bound method) are refused too. Applying a helper reached Numba as a UDF: A.apply(indexunary.value) before: UdfParseError: unary.value does not work with (INT64): Untyped global name '_resolve_expr': ... after: TypeError: indexunary.value is a helper, not an operator: it takes a comparison and returns its expression, as in indexunary.value(A > 3). To apply an op, give one such as indexunary.valuegt. --- graphblas/core/operator/utils.py | 21 ++++++++++++++++++--- graphblas/tests/test_op.py | 17 +++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/graphblas/core/operator/utils.py b/graphblas/core/operator/utils.py index 14b553353..a543eda97 100644 --- a/graphblas/core/operator/utils.py +++ b/graphblas/core/operator/utils.py @@ -186,6 +186,14 @@ def get_typed_op(op, dtype, dtype2=None, *, is_left_scalar=False, is_right_scala kind=kind, ) if isinstance(op, FunctionType): + if op.__module__ in {"graphblas.indexunary", "graphblas.select"}: + # Not a UDF: a helper such as ``indexunary.value`` or a module function + modname = op.__module__.rpartition(".")[2] + raise TypeError( + f"{modname}.{op.__name__} is a helper, not an operator: it takes a comparison " + f"and returns its expression, as in {modname}.value(A > 3). To apply an op, " + f"give one such as {modname}.valuegt." + ) if kind == "unary": op = UnaryOp.register_anonymous(op, is_udt=True) return op._compile_udt(dtype, dtype2) @@ -505,14 +513,21 @@ def _from_string(string, module, mapping, example): cur = None break op = getattr(cur, attr, None) - if op is None: - name = module.__name__.split(".")[-1] - raise ValueError(f"Unknown {name} string: {string!r}. Example usage: {example!r}") + if not _is_operator(op): + # A namespace also holds functions (``indexunary.value``), methods and modules + name = module.__name__.split(".")[-1] + raise ValueError(f"Unknown {name} string: {string!r}. Example usage: {example!r}") if dtype: op = op[dtype] return op +def _is_operator(obj): + return isinstance(obj, (OpBase, TypedOpBase, ParameterizedUdf)) or ( + _Aggregator is not None and isinstance(obj, (_Aggregator, _TypedAggregator)) + ) + + def unary_from_string(string): """Look up a UnaryOp by name or symbol, optionally typed with ``"[dtype]"``. diff --git a/graphblas/tests/test_op.py b/graphblas/tests/test_op.py index 36aa8a5b2..d44429f84 100644 --- a/graphblas/tests/test_op.py +++ b/graphblas/tests/test_op.py @@ -1291,6 +1291,23 @@ def test_from_string(): assert select.from_string(">=") is select.valuege assert indexunary.from_string("rowindex") is indexunary.rowindex assert indexunary.from_string("rowindex[int]") is indexunary.rowindex[int] + # Only operators: a namespace's helper functions and methods are not strings for one + for ns, name in [ + (indexunary, "value"), + (indexunary, "row"), + (indexunary, "column[int]"), + (indexunary, "_resolve_expr"), + (select, "value"), + (select, "register_new"), + (op, "value"), + (op, "register_new"), + ]: + with pytest.raises(ValueError, match="Unknown .* string"): + ns.from_string(name) + A = Matrix.from_coo([0], [0], [1]) + for helper in [indexunary.value, indexunary.row, select.column]: + with pytest.raises(TypeError, match=r"\.(value|row|column) is a helper, not an operator"): + A.apply(helper) # Every namespace's from_string carries a docstring (GH #513) for ns in [unary, binary, monoid, semiring, select, indexunary, agg, op]: From a836a16115cd5582afecd8c6c500bd89a754a68d Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 3 Nov 2026 12:19:47 -0800 Subject: [PATCH 06/15] Check the comparison given to the value, row and column helpers _resolve_index_expr took any expression, so a wrong one failed somewhere inside, or reached the op it picked, and a Scalar's comparison crashed in the < and >= shift, which read the thunk before taking it out of its Vector: select.row(s < 1), indexunary.row(s >= 0.5) (s a Scalar) before: AttributeError: 'Vector' object has no attribute 'value' after: s, and False indexunary.value(abs(A)) before: IndexError: list index out of range indexunary.value(A.mxm(A)) before: AttributeError: 'Matrix' object ... indexunary.value(A.apply(gt, left=3)) before: TypeError: Do not pass `left` ... indexunary.value(A.apply(plus, 3)) before: ValueError: Unknown or unregistered indexunary method: valueplus after: TypeError: indexunary.value takes a comparison of a Vector, Matrix or Scalar with a scalar on its right, as in indexunary.value(x <= 5); got M_0.apply(unary.abs[INT64]) indexunary.row(A == 3) before: ValueError: Unknown or unregistered indexunary method: roweq after: ValueError: indexunary.row compares indices with <, <=, > or >=, not == indexunary.row(A < Scalar(float)) (an empty thunk) before: TypeError: unsupported operand type(s) for -=: 'NoneType' and 'int' after: EmptyObject, as A.apply(indexunary.rowle, Scalar(float)) raises _check_index_expr accepts a built-in comparison applied with a scalar on the right (a Scalar's is an ewise_mult of two Vectors), and for row and column only the four orderings. The shift now runs after the Scalar unwrap and leaves an empty thunk to the op. With these checks the "Unknown or unregistered" raise is unreachable again, so its pragma stands. Select's helpers share the function and change the same way; main's select had the same crashes. --- graphblas/core/operator/utils.py | 62 ++++++++++++++++++++++++-------- graphblas/tests/test_matrix.py | 23 +++++++++--- graphblas/tests/test_scalar.py | 8 +++++ 3 files changed, 74 insertions(+), 19 deletions(-) diff --git a/graphblas/core/operator/utils.py b/graphblas/core/operator/utils.py index a543eda97..8f399ed45 100644 --- a/graphblas/core/operator/utils.py +++ b/graphblas/core/operator/utils.py @@ -357,34 +357,66 @@ def _resolve_index_expr(ns, modname, expr, callname, opname): f"Expected ScalarExpression, VectorExpression, or MatrixExpression; " f"found {type(expr)}\nTypical usage: {modname}.{callname}(x <= 5)" ) + _check_index_expr(modname, expr, callname, opname) tensor = expr.args[0] thunk = expr.args[1] method = f"{opname}{expr.op.name}" - if method not in ns: - # TODO: remove this once rowlt/rowge/collt/colge exist - # Convert thunk to Python int to avoid possible subtraction with uints - thunk = thunk.value - # Attempt to convert < into <= (rowlt is not part of official spec, but rowle is) - if expr.op.name == "lt": - method = f"{opname}le" - thunk -= 1 - # Attempt to convert >= into > (rowge is not part of official spec, but rowgt is) - elif expr.op.name == "ge": - method = f"{opname}gt" - thunk -= 1 - if method not in ns: # pragma: no cover (sanity) - raise ValueError(f"Unknown or unregistered {modname} method: {method}") if expr._is_scalar: - # Handle ScalarExpressions that change their arguments to Vector + # Handle ScalarExpressions that change their arguments to Vector, before + # the shift below reads the thunk's value if tensor._parent is not None: # e.g., suitesparse tensor = tensor._parent thunk = thunk._parent else: # e.g., suitesparse-vanilla tensor = tensor[0].new() thunk = thunk[0].new() + if method not in ns: + # TODO: remove this once rowlt/rowge/collt/colge exist + # Attempt to convert < into <= and >= into > (rowlt and rowge are not part of + # the official spec, but rowle and rowgt are). Convert thunk to Python int to + # avoid possible subtraction with uints. An empty thunk goes to the op as it + # is, which raises EmptyObject. + if expr.op.name in {"lt", "ge"}: + method = f"{opname}{'le' if expr.op.name == 'lt' else 'gt'}" + if not thunk._is_empty: + thunk = thunk.value - 1 + if method not in ns: # pragma: no cover (sanity) + raise ValueError(f"Unknown or unregistered {modname} method: {method}") return ns[method](tensor, thunk) +# What each helper compares: indices only by order, values by order or equality. +_INDEX_COMPARISONS = {"lt": "<", "le": "<=", "gt": ">", "ge": ">="} +_VALUE_COMPARISONS = {**_INDEX_COMPARISONS, "eq": "==", "ne": "!="} + + +def _check_index_expr(modname, expr, callname, opname): + """Raise unless ``expr`` is a comparison the ``callname`` helper can resolve. + + That is ``x OP k``: a built-in comparison applied to a Vector, Matrix or + Scalar ``x`` with a scalar ``k`` on its right, with OP one of ``<``, ``<=``, + ``>``, ``>=`` (and ``==``, ``!=`` for ``value``). A Scalar compares as the + ewise_mult of two Vectors of size 1. + """ + name = getattr(expr.op, "name", None) + if ( + expr.method_name not in ({"apply", "ewise_mult"} if expr._is_scalar else {"apply"}) + or len(expr.args) != 2 + or name not in _VALUE_COMPARISONS + or getattr(expr.op, "parent", None) is not getattr(binary, name) + or "left=" in expr.expr_repr + ): + raise TypeError( + f"{modname}.{callname} takes a comparison of a Vector, Matrix or Scalar with a " + f"scalar on its right, as in {modname}.{callname}(x <= 5); got {expr._format_expr()}" + ) + if opname != "value" and name not in _INDEX_COMPARISONS: + raise ValueError( + f"{modname}.{callname} compares indices with <, <=, > or >=, not " + f"{_VALUE_COMPARISONS[name]}" + ) + + unary.register_new = UnaryOp.register_new unary.register_anonymous = UnaryOp.register_anonymous indexbinary.register_new = IndexBinaryOp.register_new diff --git a/graphblas/tests/test_matrix.py b/graphblas/tests/test_matrix.py index 47bd305cd..18cbd77d6 100644 --- a/graphblas/tests/test_matrix.py +++ b/graphblas/tests/test_matrix.py @@ -1382,10 +1382,25 @@ def test_indexunary_helpers(A): indexunary.value(A) with pytest.raises(TypeError, match="indexunary.row"): indexunary.row(A | A) - with pytest.raises(ValueError, match="roweq"): - indexunary.row(A == 3) - with pytest.raises(ValueError, match="coleq"): - indexunary.column(A == 3) + for helper, ns in [(indexunary.row, "indexunary"), (select.column, "select")]: + with pytest.raises(ValueError, match=f"{ns}.(row|column) compares indices with <, <="): + helper(A == 3) + # Anything but a comparison with a scalar on the right is refused by name. These + # raised IndexError, AttributeError, or the error of the op the helper picked. + for expr in [ + abs(A), + A.mxm(A), + A.apply(binary.gt, left=3), + A.apply(binary.plus, right=3), + A.apply(binary.isgt, right=3), + A.ewise_mult(A, binary.gt), + ]: + for helper in [indexunary.value, select.value, indexunary.row]: + with pytest.raises(TypeError, match=r"takes a comparison .* with a scalar on its"): + helper(expr) + # An empty Scalar thunk raises as it does for the op; it raised from the shift. + with pytest.raises(EmptyObject): + indexunary.row(A < Scalar(float)).new() @autocompute diff --git a/graphblas/tests/test_scalar.py b/graphblas/tests/test_scalar.py index e1136e07f..71574543c 100644 --- a/graphblas/tests/test_scalar.py +++ b/graphblas/tests/test_scalar.py @@ -827,6 +827,14 @@ def test_select(s): assert select.valuene(5, s).new().is_empty with pytest.raises(TypeError): select.value(s | s) + # An index helper reads the thunk after taking the Scalars out of their Vectors; + # it read it from a Vector (AttributeError: 'Vector' object has no attribute 'value'). + assert select.row(s < 1).new() == s + assert select.row(s >= 1).new().is_empty + assert gb.indexunary.row(s < 1).new().value is True + assert gb.indexunary.row(s >= 0.5).new().value is False + with pytest.raises(TypeError, match="select.value takes a comparison"): + select.value(s + 1) @pytest.mark.skipif("not suitesparse") From 1a1ab82e342650bee67aa39f7e5e8c7884e62955 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 3 Nov 2026 12:20:36 -0800 Subject: [PATCH 07/15] Find a lazily registered op from the index helpers, as attribute access does _resolve_index_expr looked the op up in the namespace's globals, which hold an op registered with lazy=True only once something touches it: indexunary.register_new("rowlt", lambda x, i, j, t: i < t, lazy=True) indexunary.row(A < 2.5) (rows 0 to 3) before: [True, True, False, False] (the <= shift), then [True, True, True, False] once indexunary.rowlt was touched after: [True, True, True, False] both times, from the op _namespace_op builds such an op through the module's __getattr__, as indexunary.rowlt would. select.row does the same; main's select did too. --- graphblas/core/operator/utils.py | 17 ++++++++++++++--- graphblas/tests/test_matrix.py | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/graphblas/core/operator/utils.py b/graphblas/core/operator/utils.py index 8f399ed45..727175785 100644 --- a/graphblas/core/operator/utils.py +++ b/graphblas/core/operator/utils.py @@ -370,7 +370,7 @@ def _resolve_index_expr(ns, modname, expr, callname, opname): else: # e.g., suitesparse-vanilla tensor = tensor[0].new() thunk = thunk[0].new() - if method not in ns: + if _namespace_op(ns, method) is None: # TODO: remove this once rowlt/rowge/collt/colge exist # Attempt to convert < into <= and >= into > (rowlt and rowge are not part of # the official spec, but rowle and rowgt are). Convert thunk to Python int to @@ -380,9 +380,20 @@ def _resolve_index_expr(ns, modname, expr, callname, opname): method = f"{opname}{'le' if expr.op.name == 'lt' else 'gt'}" if not thunk._is_empty: thunk = thunk.value - 1 - if method not in ns: # pragma: no cover (sanity) + if _namespace_op(ns, method) is None: # pragma: no cover (sanity) raise ValueError(f"Unknown or unregistered {modname} method: {method}") - return ns[method](tensor, thunk) + return _namespace_op(ns, method)(tensor, thunk) + + +def _namespace_op(ns, name): + """The op ``name`` in namespace globals ``ns``, or None; it builds one registered lazily. + + A lazily registered op waits in the namespace's ``_delayed`` until its module + ``__getattr__`` builds it, so ``name in ns`` alone misses it until then. + """ + if name not in ns and name in ns.get("_delayed", ()): + ns["__getattr__"](name) + return ns.get(name) # What each helper compares: indices only by order, values by order or equality. diff --git a/graphblas/tests/test_matrix.py b/graphblas/tests/test_matrix.py index 18cbd77d6..17ecdf694 100644 --- a/graphblas/tests/test_matrix.py +++ b/graphblas/tests/test_matrix.py @@ -1403,6 +1403,25 @@ def test_indexunary_helpers(A): indexunary.row(A < Scalar(float)).new() +@pytest.mark.skipif("not supports_udfs") +def test_index_helpers_find_a_lazily_registered_op(): + """A helper uses an op registered with ``lazy=True`` before anything touches it. + + It looked only at the namespace's built ops, so ``row(A < 2.5)`` took the + ``<=`` shift until something touched ``indexunary.rowlt``, then the op. + """ + A = Matrix.from_coo([0, 1, 2, 3], [0, 0, 0, 0], [1, 1, 1, 1]) + try: + indexunary.register_new("rowlt", lambda x, i, j, t: i < t, lazy=True) + for _ in range(2): + assert indexunary.row(A < 2.5).new().to_coo()[2].tolist() == [True] * 3 + [False] + assert select.row(A < 2.5).new().to_coo()[0].tolist() == [0, 1, 2] + finally: + for ns in [indexunary, select]: + ns._delayed.pop("rowlt", None) + vars(ns).pop("rowlt", None) + + @autocompute def test_select_by_a_transposed_comparison(): """``A.select(A.T == x)`` keeps the entries where ``A.T == x``, as a computed mask does. From a52d39d9f860ae428825279a7e55e3a1162efcd2 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 3 Nov 2026 12:21:03 -0800 Subject: [PATCH 08/15] Say why get_typed_op can skip the Aggregator check before registration The comment said no Aggregator can exist before agg.py registers its classes, but agg.py builds every built-in Aggregator at module level first. What holds is that none reaches get_typed_op until then. --- graphblas/core/operator/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/graphblas/core/operator/utils.py b/graphblas/core/operator/utils.py index 727175785..aa92db452 100644 --- a/graphblas/core/operator/utils.py +++ b/graphblas/core/operator/utils.py @@ -74,7 +74,8 @@ # ``agg.py`` imports ``get_typed_op`` from this module, so the two form an import # cycle and we can't import the Aggregator classes at module load. ``agg.py`` # registers them here when it loads, which ``core.operator.__init__`` does -# eagerly at import. Before registration no Aggregator instance can exist, so +# eagerly at import. agg.py creates its Aggregators before it registers the +# classes, but none reaches ``get_typed_op`` until then, so before registration # ``get_typed_op`` skips the check entirely. _Aggregator = None _TypedAggregator = None From 9f23538527dce428fd8d548cc792a2b7af546b54 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 3 Nov 2026 12:21:05 -0800 Subject: [PATCH 09/15] Test the example in indexunary.from_string's error message "Add docstrings to the operator from_string helpers (GH #513)" fixed the example in that message (row_index, which does not resolve, became rowindex) without a test; on its parent this one fails. --- graphblas/tests/test_op.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/graphblas/tests/test_op.py b/graphblas/tests/test_op.py index d44429f84..ea39ac5ac 100644 --- a/graphblas/tests/test_op.py +++ b/graphblas/tests/test_op.py @@ -1291,6 +1291,9 @@ def test_from_string(): assert select.from_string(">=") is select.valuege assert indexunary.from_string("rowindex") is indexunary.rowindex assert indexunary.from_string("rowindex[int]") is indexunary.rowindex[int] + # The error's example is an op that resolves (it was 'row_index') + with pytest.raises(ValueError, match="Example usage: 'rowindex'"): + indexunary.from_string("bad_indexunary") # Only operators: a namespace's helper functions and methods are not strings for one for ns, name in [ (indexunary, "value"), From 51b1f5cc3c75fd9fac798ccd4a22efdda7cd6d10 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 3 Nov 2026 12:21:22 -0800 Subject: [PATCH 10/15] Say in indexunary.row's docstring that indexunary.index is an op select.index(v < 4) filters by index, but indexunary.index is the rowindex op (a helper would shadow it), so indexunary.index(v < 4) gives each entry's index without an error. The note for that was a code comment; the row helper's docstring now says to use row for a Vector. --- graphblas/indexunary/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/graphblas/indexunary/__init__.py b/graphblas/indexunary/__init__.py index 4aee3524a..40fe88eae 100644 --- a/graphblas/indexunary/__init__.py +++ b/graphblas/indexunary/__init__.py @@ -56,6 +56,11 @@ def row(expr): The example will dispatch to ``gb.indexunary.rowle(A, 5)`` while being potentially nicer to read. + + For a Vector, ``gb.indexunary.row(v < 4)`` compares indices. Unlike + ``gb.select.index``, ``gb.indexunary.index`` is not a helper but the + ``rowindex`` op, so ``gb.indexunary.index(v < 4)`` gives the index of each + entry of ``v < 4``. """ return _resolve_expr(expr, "row", "row") From cd12ce75bff827c088a80bca6bc8401141cfb0d7 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 3 Nov 2026 14:07:33 -0800 Subject: [PATCH 11/15] Compare indices with the bound given in the row and column helpers Port of PR A's "Compare indices with the bound given in select.row, column and index" to _resolve_index_expr, which the select and indexunary helpers share here. When #604's chain is replayed over A, its "Add indexunary.value/row/column helpers accepting comparison expressions (gh-239)" conflicts in graphblas/select/__init__.py; take that commit's own version of the file (select delegates here), and this commit carries A's fix into the shared function. The helpers read the thunk from the comparison x < k, which types k beside x's values: beside INT8 or UINT8, 300 is infinity, and beside FP32, 2**24 + 1 rounds. indexunary.row(B <= 299) on a 400-row INT8 matrix was True in all 400 rows, as select.row(B < 300) kept them. Now they compare indices with the number the caller gave (the expression's _right_literal, from A), or the thunk Scalar's value, made a whole number: i < t is i <= ceil(t) - 1, i >= t is i > ceil(t) - 1, and i <= t and i > t take floor(t). An op registered for < or >= (a user's rowlt) gets the number as given. An empty Scalar thunk still reaches the op, which raises EmptyObject. Also r604's R1: a Scalar's .apply keeps its thunk a Scalar, so taking it out of a Vector crashed: select.value(s.apply(binary.lt, right=10)) raised AttributeError on suitesparse and TypeError on vanilla. The thunk is now taken out of a Vector only when it is one. Tests: indexunary.row and column on Matrix (INT8, UINT8, FP32, transposed, fractional bounds), Vector and Scalar, and the R1 case. --- graphblas/core/operator/utils.py | 61 +++++++++++++++++++++++--------- graphblas/tests/test_matrix.py | 21 +++++++++++ graphblas/tests/test_scalar.py | 9 +++++ graphblas/tests/test_vector.py | 4 +++ 4 files changed, 78 insertions(+), 17 deletions(-) diff --git a/graphblas/core/operator/utils.py b/graphblas/core/operator/utils.py index aa92db452..3f590e3e6 100644 --- a/graphblas/core/operator/utils.py +++ b/graphblas/core/operator/utils.py @@ -1,3 +1,5 @@ +import math +import numbers from types import BuiltinFunctionType, FunctionType, ModuleType from ... import ( @@ -361,31 +363,56 @@ def _resolve_index_expr(ns, modname, expr, callname, opname): _check_index_expr(modname, expr, callname, opname) tensor = expr.args[0] thunk = expr.args[1] - method = f"{opname}{expr.op.name}" if expr._is_scalar: - # Handle ScalarExpressions that change their arguments to Vector, before - # the shift below reads the thunk's value + # Handle ScalarExpressions that change their arguments to Vector, before the + # bound below reads the thunk's value. A comparison (an ewise_mult) has both + # as Vectors of size 1; an apply keeps its thunk a Scalar. if tensor._parent is not None: # e.g., suitesparse tensor = tensor._parent - thunk = thunk._parent + if not thunk._is_scalar: + thunk = thunk._parent else: # e.g., suitesparse-vanilla tensor = tensor[0].new() - thunk = thunk[0].new() - if _namespace_op(ns, method) is None: - # TODO: remove this once rowlt/rowge/collt/colge exist - # Attempt to convert < into <= and >= into > (rowlt and rowge are not part of - # the official spec, but rowle and rowgt are). Convert thunk to Python int to - # avoid possible subtraction with uints. An empty thunk goes to the op as it - # is, which raises EmptyObject. - if expr.op.name in {"lt", "ge"}: - method = f"{opname}{'le' if expr.op.name == 'lt' else 'gt'}" - if not thunk._is_empty: - thunk = thunk.value - 1 - if _namespace_op(ns, method) is None: # pragma: no cover (sanity) - raise ValueError(f"Unknown or unregistered {modname} method: {method}") + if not thunk._is_scalar: + thunk = thunk[0].new() + name = expr.op.name + if opname != "value": + if name in {"lt", "ge"} and _namespace_op(ns, f"{opname}{name}") is not None: + # An op registered for it (a user's rowlt): the number as given + if expr._right_literal is not None: + thunk = expr._right_literal + else: + thunk, name = _index_bound(expr._right_literal, thunk, name) + method = f"{opname}{name}" + if _namespace_op(ns, method) is None: # pragma: no cover (sanity) + raise ValueError(f"Unknown or unregistered {modname} method: {method}") return _namespace_op(ns, method)(tensor, thunk) +def _index_bound(literal, thunk, name): + """Return the thunk and comparison that select an index ``name`` (lt, le, gt, ge) a bound. + + The bound is the number the caller gave (``literal``), else the thunk Scalar's value. + The comparison typed its thunk beside the values, which is wrong for an index: 300 + beside INT8 values is infinity, and 2**24 + 1 beside FP32 rounds. Only rowle and + rowgt exist (and colle, colgt, indexle, indexgt), and indices are whole numbers, so + ``i < t`` is ``i <= ceil(t) - 1``, ``i >= t`` is ``i > ceil(t) - 1``, and ``i <= t`` + and ``i > t`` take ``floor(t)``, where GraphBLAS would truncate a float toward zero + (``i > -0.5`` dropped index 0). A whole number beyond INT64, which holds every index, + is clamped to it. An empty Scalar goes to the op as it is, which raises EmptyObject. + """ + shift = name in {"lt", "ge"} + if shift: + name = "le" if name == "lt" else "gt" + bound = thunk.value if literal is None else literal + if not isinstance(bound, numbers.Real): + return thunk, name + if math.isfinite(bound): + bound = math.ceil(bound) - 1 if shift else math.floor(bound) + bound = min(max(bound, -(2**63)), 2**63 - 1) + return bound, name + + def _namespace_op(ns, name): """The op ``name`` in namespace globals ``ns``, or None; it builds one registered lazily. diff --git a/graphblas/tests/test_matrix.py b/graphblas/tests/test_matrix.py index 17ecdf694..38ea47117 100644 --- a/graphblas/tests/test_matrix.py +++ b/graphblas/tests/test_matrix.py @@ -1401,6 +1401,27 @@ def test_indexunary_helpers(A): # An empty Scalar thunk raises as it does for the op; it raised from the shift. with pytest.raises(EmptyObject): indexunary.row(A < Scalar(float)).new() + # Indices are compared with the bound given, as a whole number, not with the thunk + # typed beside the values: beside INT8 or UINT8, 300 is infinity, and beside FP32, + # 2**24 + 1 rounds (as for select's helpers). + n = 400 + for dtype in ["INT8", "UINT8"]: + B = Matrix.from_coo(np.arange(n), np.zeros(n, int), 1, dtype=dtype, nrows=n, ncols=1) + for expr, count in [(B < 300, 300), (B > 299, 100), (B <= 299, 300), (B >= 300, 100)]: + assert indexunary.row(expr).new().to_coo()[2].sum() == count + assert indexunary.column(B.T <= 299).new().to_coo()[2].sum() == 300 + rows = [2**24 - 1, 2**24, 2**24 + 1] + F = Matrix.from_coo(rows, [0, 0, 0], [1.0, 2.0, 3.0], dtype="FP32", nrows=2**24 + 3, ncols=1) + assert indexunary.row(F < 2**24 + 1).new().to_coo()[2].tolist() == [True, True, False] + A5 = Matrix.from_coo(np.arange(5), np.zeros(5, int), 1.0, nrows=5, ncols=1) + for expr, expected in [ + (A5 < A5.nrows / 2, [True, True, True, False, False]), + (A5 >= 2.5, [False, False, False, True, True]), + (A5 > -0.5, [True] * 5), + (A5 <= 2.5, [True, True, True, False, False]), + ]: + assert indexunary.row(expr).new().to_coo()[2].tolist() == expected + assert indexunary.column(A5.T < 2.5).new().to_coo()[2].tolist() == [True] * 3 + [False] * 2 @pytest.mark.skipif("not supports_udfs") diff --git a/graphblas/tests/test_scalar.py b/graphblas/tests/test_scalar.py index 71574543c..596de8802 100644 --- a/graphblas/tests/test_scalar.py +++ b/graphblas/tests/test_scalar.py @@ -835,6 +835,15 @@ def test_select(s): assert gb.indexunary.row(s >= 0.5).new().value is False with pytest.raises(TypeError, match="select.value takes a comparison"): select.value(s + 1) + # A Scalar's apply keeps its thunk a Scalar (AttributeError on suitesparse, and + # TypeError on vanilla, from taking it out of a Vector) + assert select.value(s.apply(binary.lt, right=10)).new() == s + assert gb.indexunary.row(s.apply(binary.gt, right=-0.5)).new().value is True + # The bound given, as a whole number, not the thunk typed beside the value + assert gb.indexunary.row(s > -0.5).new().value is True + s8 = Scalar.from_value(1, dtype=dtypes.INT8) + assert gb.indexunary.row(s8 < 300).new().value is True + assert gb.indexunary.row(s8 > 299).new().value is False @pytest.mark.skipif("not suitesparse") diff --git a/graphblas/tests/test_vector.py b/graphblas/tests/test_vector.py index 4fb3f91d3..deac8cc85 100644 --- a/graphblas/tests/test_vector.py +++ b/graphblas/tests/test_vector.py @@ -820,6 +820,10 @@ def test_indexunary_helpers(v): assert indexunary.row(v >= 4).new().isequal(v.apply(indexunary.rowgt, 3).new()) assert indexunary.value(v == 1).new().dtype == dtypes.BOOL assert indexunary.row(v < 4).new().dtype == dtypes.BOOL + # The index is compared with the bound given, not with 300 typed beside INT8 (inf) + w = Vector.from_coo([0, 200, 399], 1, dtype=dtypes.INT8, size=400) + assert indexunary.row(w < 300).new().to_coo()[1].tolist() == [True, True, False] + assert indexunary.row(w > 2.5).new().to_coo()[1].tolist() == [False, True, True] # Scalar expression path resolves to a ScalarExpression, like select.value. s = Scalar.from_value(1) assert indexunary.value(s < 10).new() == s.apply(indexunary.valuelt, 10).new() From 8c370b8c8bd3b760684b12b2cdb7ac219f7d60cb Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 3 Nov 2026 14:22:59 -0800 Subject: [PATCH 12/15] Take the thunk's value as an index bound that is not a number, here too The same change as PR A's "Take the thunk's value as an index bound that is not a number", in _index_bound here, which the select and indexunary helpers share: indexunary.row(A < np.array(3)) kept rows 0 to 3. --- graphblas/core/operator/utils.py | 4 +++- graphblas/tests/test_matrix.py | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/graphblas/core/operator/utils.py b/graphblas/core/operator/utils.py index 3f590e3e6..b3bfb14ae 100644 --- a/graphblas/core/operator/utils.py +++ b/graphblas/core/operator/utils.py @@ -404,7 +404,9 @@ def _index_bound(literal, thunk, name): shift = name in {"lt", "ge"} if shift: name = "le" if name == "lt" else "gt" - bound = thunk.value if literal is None else literal + # The caller's number, if it is one; a Scalar expression (``w[0]``), a 0-d array or + # ``np.True_`` made the thunk keep its own type, so the thunk's value is the bound. + bound = literal if isinstance(literal, numbers.Real) else thunk.value if not isinstance(bound, numbers.Real): return thunk, name if math.isfinite(bound): diff --git a/graphblas/tests/test_matrix.py b/graphblas/tests/test_matrix.py index 38ea47117..a68c7a8fc 100644 --- a/graphblas/tests/test_matrix.py +++ b/graphblas/tests/test_matrix.py @@ -1419,6 +1419,9 @@ def test_indexunary_helpers(A): (A5 >= 2.5, [False, False, False, True, True]), (A5 > -0.5, [True] * 5), (A5 <= 2.5, [True, True, True, False, False]), + # A bound that is not a Python or numpy number: the thunk keeps its own type + (A5 < np.array(3), [True, True, True, False, False]), + (A5 >= np.array(3), [False, False, False, True, True]), ]: assert indexunary.row(expr).new().to_coo()[2].tolist() == expected assert indexunary.column(A5.T < 2.5).new().to_coo()[2].tolist() == [True] * 3 + [False] * 2 From 71e408736949d8fc27bb6ae342b5ae05dc0fc228 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 3 Nov 2026 15:26:13 -0800 Subject: [PATCH 13/15] Find a lazily registered IndexUnaryOp from select's helpers too An IndexUnaryOp registered with lazy=True waits in indexunary._delayed, and its SelectOp is made only when it is built. select's helpers looked in select's namespace alone, so select.row(A < 2.5) took the <= shift until something touched indexunary.rowlt, and then the op (r604 R3). _namespace_op now builds the indexunary op for select when select has none by that name, as attribute access on indexunary would. The test of a lazily registered op could not fail since the helpers compare indices with the bound given: its rowlt, i < t, answered as the <= shift does, so it passed with the lazy lookup removed (r604 P1). Its rowlt is now i == 0, which the shift does not give, and select.row runs first. It fails with either the lazy lookup or this change taken out. --- graphblas/core/operator/utils.py | 11 ++++++++--- graphblas/tests/test_matrix.py | 11 +++++++---- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/graphblas/core/operator/utils.py b/graphblas/core/operator/utils.py index b3bfb14ae..c84c6db5b 100644 --- a/graphblas/core/operator/utils.py +++ b/graphblas/core/operator/utils.py @@ -419,10 +419,15 @@ def _namespace_op(ns, name): """The op ``name`` in namespace globals ``ns``, or None; it builds one registered lazily. A lazily registered op waits in the namespace's ``_delayed`` until its module - ``__getattr__`` builds it, so ``name in ns`` alone misses it until then. + ``__getattr__`` builds it, so ``name in ns`` alone misses it until then. An + IndexUnaryOp registered lazily makes its SelectOp only once it is built, so select + builds it from indexunary's ``_delayed``. """ - if name not in ns and name in ns.get("_delayed", ()): - ns["__getattr__"](name) + if name not in ns: + if name in ns.get("_delayed", ()): + ns["__getattr__"](name) + elif ns is vars(select) and name in indexunary._delayed: + getattr(indexunary, name) return ns.get(name) diff --git a/graphblas/tests/test_matrix.py b/graphblas/tests/test_matrix.py index a68c7a8fc..9a33c5131 100644 --- a/graphblas/tests/test_matrix.py +++ b/graphblas/tests/test_matrix.py @@ -1432,14 +1432,17 @@ def test_index_helpers_find_a_lazily_registered_op(): """A helper uses an op registered with ``lazy=True`` before anything touches it. It looked only at the namespace's built ops, so ``row(A < 2.5)`` took the - ``<=`` shift until something touched ``indexunary.rowlt``, then the op. + ``<=`` shift until something touched ``indexunary.rowlt``, then the op. select + has a SelectOp for it only once the indexunary op is built, so ``select.row`` + took the shift until ``indexunary.rowlt`` was touched. """ A = Matrix.from_coo([0, 1, 2, 3], [0, 0, 0, 0], [1, 1, 1, 1]) try: - indexunary.register_new("rowlt", lambda x, i, j, t: i < t, lazy=True) + # It answers otherwise than the <= shift, so the result shows which ran. + indexunary.register_new("rowlt", lambda x, i, j, t: i == 0, lazy=True) for _ in range(2): - assert indexunary.row(A < 2.5).new().to_coo()[2].tolist() == [True] * 3 + [False] - assert select.row(A < 2.5).new().to_coo()[0].tolist() == [0, 1, 2] + assert select.row(A < 2.5).new().to_coo()[0].tolist() == [0] + assert indexunary.row(A < 2.5).new().to_coo()[2].tolist() == [True] + [False] * 3 finally: for ns in [indexunary, select]: ns._delayed.pop("rowlt", None) From bb787ffc0dbd9feae98695018c20979ed765e056 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 3 Nov 2026 15:36:37 -0800 Subject: [PATCH 14/15] Select no index for a NaN bound in the row, column and index helpers, here too The same change as on #661, in _index_bound's twin in core/operator/utils.py, which the select and indexunary helpers share here: A of 5 rows: indexunary.row(A < nan), (A <= nan) before: True for row 0 only indexunary.row(A > nan), (A >= nan) before: True for rows 1 to 4 after: False for every row, and select.row keeps none (main kept row 0, or rows 1 to 4) _index_bound also reads a value only from a Scalar thunk now, as on #661, where that restored the op's TypeError for A.apply(binary.le, left=3). Here _check_index_expr refuses that form first, so the two copies stay alike without a test of it. --- graphblas/core/operator/utils.py | 13 +++++++++++-- graphblas/tests/test_matrix.py | 6 ++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/graphblas/core/operator/utils.py b/graphblas/core/operator/utils.py index c84c6db5b..a4d727aab 100644 --- a/graphblas/core/operator/utils.py +++ b/graphblas/core/operator/utils.py @@ -399,16 +399,25 @@ def _index_bound(literal, thunk, name): ``i < t`` is ``i <= ceil(t) - 1``, ``i >= t`` is ``i > ceil(t) - 1``, and ``i <= t`` and ``i > t`` take ``floor(t)``, where GraphBLAS would truncate a float toward zero (``i > -0.5`` dropped index 0). A whole number beyond INT64, which holds every index, - is clamped to it. An empty Scalar goes to the op as it is, which raises EmptyObject. + is clamped to it. NaN compares False with every index, so it keeps none. An empty + Scalar goes to the op as it is, which raises EmptyObject, and so does a thunk that is + no Scalar (from ``A.apply(binary.le, left=3)``), which raises the op's TypeError. """ shift = name in {"lt", "ge"} if shift: name = "le" if name == "lt" else "gt" # The caller's number, if it is one; a Scalar expression (``w[0]``), a 0-d array or # ``np.True_`` made the thunk keep its own type, so the thunk's value is the bound. - bound = literal if isinstance(literal, numbers.Real) else thunk.value + if isinstance(literal, numbers.Real): + bound = literal + elif getattr(thunk, "_is_scalar", False): + bound = thunk.value + else: + return thunk, name if not isinstance(bound, numbers.Real): return thunk, name + if math.isnan(bound): + return (-1 if name == "le" else 2**63 - 1), name # no index is <= -1 or > INT64_MAX if math.isfinite(bound): bound = math.ceil(bound) - 1 if shift else math.floor(bound) bound = min(max(bound, -(2**63)), 2**63 - 1) diff --git a/graphblas/tests/test_matrix.py b/graphblas/tests/test_matrix.py index 9a33c5131..99d404311 100644 --- a/graphblas/tests/test_matrix.py +++ b/graphblas/tests/test_matrix.py @@ -1425,6 +1425,12 @@ def test_indexunary_helpers(A): ]: assert indexunary.row(expr).new().to_coo()[2].tolist() == expected assert indexunary.column(A5.T < 2.5).new().to_coo()[2].tolist() == [True] * 3 + [False] * 2 + # NaN compares False with every index; cast to the INT64 thunk it was 0, so < and <= + # gave True for row 0, and > and >= for every other row (select's helpers too) + nan = float("nan") + for expr in [A5 < nan, A5 <= nan, A5 > nan, A5 >= nan, A5 < Scalar.from_value(nan)]: + assert indexunary.row(expr).new().to_coo()[2].tolist() == [False] * 5, expr + assert select.row(expr).new().nvals == 0, expr @pytest.mark.skipif("not supports_udfs") From 44fee278eebf2f8e4a982dc0f33e67bf280a5b8b Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Wed, 4 Nov 2026 10:33:47 -0800 Subject: [PATCH 15/15] Expect select.row's own TypeError for a left-operand comparison here #661's test of select.row(A.apply(binary.le, left=3)) expects the op's TypeError, as main gives. Here the helpers refuse such an expression first, with a TypeError that names the helper ("Check the comparison given to the value, row and column helpers"), so the test now expects that message. --- graphblas/tests/test_matrix.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/graphblas/tests/test_matrix.py b/graphblas/tests/test_matrix.py index 99d404311..622b7e389 100644 --- a/graphblas/tests/test_matrix.py +++ b/graphblas/tests/test_matrix.py @@ -1550,9 +1550,10 @@ def test_select_row_and_column_compare_indices_with_the_bound_given(): with gb.config.set(autocompute=True): assert select.row(A < w[0]).new().to_coo()[0].tolist() == [0, 1, 2] assert select.row(A >= w[0]).new().to_coo()[0].tolist() == [3, 4] - # A comparison with the scalar on the left gives the op's TypeError, as on main; it - # was AttributeError ('Matrix' object has no attribute 'value') - with pytest.raises(TypeError, match="Bad type for keyword argument `thunk=`"): + # A comparison with the scalar on the left is refused with a TypeError that names + # the helper (main gave the op's TypeError; it was briefly AttributeError: 'Matrix' + # object has no attribute 'value') + with pytest.raises(TypeError, match="select.row takes a comparison"): select.row(A.apply(binary.le, left=3)) # NaN compares False with every index, so it keeps none; NaN cast to the INT64 thunk # was 0, so < and <= kept row 0, and > and >= every other row (main too)