GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Reject building a user monoid from a built-in BinaryOp by eriknw · Pull Request #617 · python-graphblas/python-graphblas

Reject building a user monoid from a built-in BinaryOp #617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
eriknw merged 1 commit into main from 36-monoid-builtin-binaryop-guard
Nov 11, 2026

Uh oh!

There was an error while loading. Please reload this page.

Merged

Reject building a user monoid from a built-in BinaryOp #617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
512b8ab
Reject building a user monoid from a built-in BinaryOp
eriknw Aug 5, 2026
File filter
Filter by extension

Clear filters
Conversations
Failed to load comments.
Loading

Uh oh!

There was an error while loading. Please reload this page.

Jump to
Jump to file
Failed to load files.
Loading

Uh oh!

There was an error while loading. Please reload this page.

Diff view
Diff view

There are no files selected for viewing

28 changes: 28 additions & 0 deletions 28 graphblas/core/operator/monoid.py
Show comments
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@
_BUILTIN_UDT_MONOIDS = {"plus", "times", "min", "max"}


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.
"""
return binaryop.orig_func is None and binaryop._numba_func is None


def _scalar_identity(monoid_name, scalar_dtype):
"""Return the identity for a single numeric (or bool) numpy dtype, or ``None``.

Expand Down Expand Up @@ -262,6 +272,24 @@ class Monoid(OpBase):
def _build(cls, name, binaryop, identity, *, is_idempotent=False, anonymous=False):
if type(binaryop) is not BinaryOp:
raise TypeError(f"binaryop must be a BinaryOp, not {type(binaryop)}")
if _is_builtin_binaryop(binaryop):
Comment thread

@eriknw eriknw Nov 11, 2026

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether it's actually worth making _is_builtin_binaryop a single-line helper functions that is used in one location.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
# Rejected up front, before any object is created: a built-in binaryop
# cannot back a user monoid. Its typed ops are ``TypedBuiltinBinaryOp``
# with no ``_monoid`` slot, so ``TypedUserMonoid`` would raise an opaque
# AttributeError partway through, having already repointed
# ``binary.<name>._monoid`` at the half-built monoid. Even if that were
# patched over, SuiteSparse ignores the identity handed to
# ``GrB_Monoid_new`` whenever the built-in op already has a built-in
# monoid (max, min, plus, times, any), so the result would silently
# disagree with ``monoid[dtype].identity``.
raise TypeError(
"binaryop must be a user-defined BinaryOp, not the built-in "
f"binary.{binaryop.name}. Built-in binary operators already own their "
"monoid association (binary.plus resolves to monoid.plus, and so on). "
"To build a monoid with a different identity, register the function as "
"a new BinaryOp (BinaryOp.register_new or BinaryOp.register_anonymous) "
"and pass that."
)
if name is None:
name = binaryop.name
new_type_obj = cls(
Expand Down
18 changes: 18 additions & 0 deletions 18 graphblas/tests/test_op.py
Show comments
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,24 @@ def bad_identity(x=0):
assert monoid.is_idempotent


def test_monoid_rejects_builtin_binaryop():
"""A built-in binaryop cannot back a user monoid.

Built-ins already own their monoid association (binary.max resolves to
monoid.max), and SuiteSparse silently ignores the identity passed to
GrB_Monoid_new for them, so accepting one would produce a monoid whose
Python-side identity disagrees with what GraphBLAS computes with.
"""
with pytest.raises(TypeError, match="must be a user-defined BinaryOp"):
Monoid.register_new("_bad_builtin_monoid", binary.max, 0)
with pytest.raises(TypeError, match="must be a user-defined BinaryOp"):
Monoid.register_anonymous(binary.plus, 1)
# The rejection happens before any object is created, so the built-in's
# own monoid association is untouched.
assert binary.max.monoid is monoid.max
assert not hasattr(gb.monoid, "_bad_builtin_monoid")


@pytest.mark.skipif("not supports_udfs")
@pytest.mark.slow
def test_semiring_parameterized():
Expand Down
Loading
Oops, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.