From a30c4919468c7f5d21b8dbe5aca1f4ef222de398 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Wed, 5 Aug 2026 01:34:35 -0500 Subject: [PATCH] Extend the monoid guard to built-ins that carry a UDT fallback func The guard added with the monoid-from-built-in rejection keyed on "constructed with neither a Python func nor a numba func", which is true for most built-ins but not all of them: binary.any, first, pair, and second carry a Python orig_func as their UDT fallback while their typed ops are still TypedBuiltinBinaryOp. All four sailed past the guard and reproduced exactly what it exists to prevent: an opaque AttributeError partway through registration, with binary.._monoid left pointing at the half-built monoid (binary.any.monoid is monoid.any flipped from True to False after the rejected call). binary.any is even named in the guard's own rationale as an op whose identity SuiteSparse ignores. Key the probe on the typed ops as well: any TypedBuiltinBinaryOp among them marks the op built-in. The two signals agree everywhere else, including under mapnumpy=True, where the mapped numpy binary ops hold the built-in's typed ops and no funcs, and are flagged by both tests alike; the numpy monoids themselves are mapped without passing through Monoid._build, so their registration is unaffected (verified under both mapnumpy settings). A UDF's typed ops are never TypedBuiltinBinaryOp, so nothing legitimate is newly rejected. --- graphblas/core/operator/monoid.py | 19 +++++++++++++++---- graphblas/tests/test_op.py | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/graphblas/core/operator/monoid.py b/graphblas/core/operator/monoid.py index 47adcd140..f13a7ade9 100644 --- a/graphblas/core/operator/monoid.py +++ b/graphblas/core/operator/monoid.py @@ -32,11 +32,22 @@ def _is_builtin_binaryop(binaryop): """True if ``binaryop`` wraps built-in GraphBLAS operators rather than a UDF. - Built-ins are the only binary ops constructed with neither a Python func nor - a numba func. That covers ``binary.numpy.float_power`` too, which is - assembled out of ``binary.pow``'s typed ops. + Most built-ins are constructed with neither a Python func nor a numba func. + That covers ``binary.numpy.float_power`` too, which is assembled out of + ``binary.pow``'s typed ops. A few built-ins (``any``, ``first``, ``pair``, + ``second``) carry a Python ``orig_func`` as their UDT fallback, so the func + test alone misses them; their typed ops are still ``TypedBuiltinBinaryOp``, + which is the authoritative signal. A UDF's typed ops are never + ``TypedBuiltinBinaryOp``. Under ``mapnumpy=True`` the mapped numpy binary + ops hold the built-in's typed ops and no funcs, so both tests agree they + are built-in; the numpy monoids themselves are mapped without passing + through ``Monoid._build``. """ - return binaryop.orig_func is None and binaryop._numba_func is None + return ( + binaryop.orig_func is None + and binaryop._numba_func is None + or any(type(typed) is TypedBuiltinBinaryOp for typed in binaryop._typed_ops.values()) + ) def _scalar_identity(monoid_name, scalar_dtype): diff --git a/graphblas/tests/test_op.py b/graphblas/tests/test_op.py index f68cb60d2..d50265a0d 100644 --- a/graphblas/tests/test_op.py +++ b/graphblas/tests/test_op.py @@ -443,6 +443,24 @@ def test_monoid_rejects_builtin_binaryop(): assert not hasattr(gb.monoid, "_bad_builtin_monoid") +def test_monoid_rejects_builtin_binaryop_with_udt_fallback(): + """binary.any, first, pair, and second are built-ins that carry a Python + func as their UDT fallback, so a func-based probe alone misses them. They + must be rejected like the other built-ins: their typed ops have no _monoid + slot, and a failed registration used to leave binary.._monoid + pointing at the half-built monoid. + """ + for binop in [binary.any, binary.first, binary.pair, binary.second]: + with pytest.raises(TypeError, match="must be a user-defined BinaryOp"): + Monoid.register_anonymous(binop, 0) + # State is untouched: any's built-in monoid association survives, and the + # ops without a built-in monoid still have none. + assert binary.any.monoid is monoid.any + assert binary.first.monoid is None + assert binary.pair.monoid is None + assert binary.second.monoid is None + + @pytest.mark.skipif("not supports_udfs") @pytest.mark.slow def test_semiring_parameterized():