Skip to content

Remove deprecated require_monoid #377

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
merged 4 commits into from
Jan 28, 2023
Merged

Remove deprecated require_monoid #377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2083cce
Remove deprecated `require_monoid`
eriknw Jan 25, 2023
37249b0
alphabetical ruff
eriknw Jan 26, 2023
424febf
bump
eriknw Jan 27, 2023
9fd7149
bump
eriknw Jan 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ repos:
- id: mixed-line-ending
- id: trailing-whitespace
- repo: https://github.com/abravalheri/validate-pyproject
rev: v0.11
rev: v0.12.1
hooks:
- id: validate-pyproject
name: Validate pyproject.toml
Expand All @@ -30,7 +30,7 @@ repos:
- id: autoflake
args: [--in-place]
- repo: https://github.com/pycqa/isort
rev: 5.11.4
rev: 5.12.0
hooks:
- id: isort
- repo: https://github.com/asottile/pyupgrade
Expand Down Expand Up @@ -71,7 +71,7 @@ repos:
additional_dependencies: [tomli]
files: ^(graphblas|docs)/
- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: v0.0.233
rev: v0.0.237
hooks:
- id: ruff
- repo: https://github.com/sphinx-contrib/sphinx-lint
Expand Down
23 changes: 2 additions & 21 deletions graphblas/core/matrix.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1688,7 +1688,7 @@ def _carg(self):
# to __setitem__ to trigger a call to GraphBLAS
#########################################################

def ewise_add(self, other, op=monoid.plus, *, require_monoid=None):
def ewise_add(self, other, op=monoid.plus):
"""Perform element-wise computation on the union of sparse values,
similar to how one expects addition to work for sparse matrices.

Expand All @@ -1702,7 +1702,6 @@ def ewise_add(self, other, op=monoid.plus, *, require_monoid=None):
The other matrix in the computation
op : Monoid or BinaryOp
Operator to use on intersecting values
require_monoid : deprecated

Returns
-------
Expand All @@ -1718,14 +1717,6 @@ def ewise_add(self, other, op=monoid.plus, *, require_monoid=None):
# Functional syntax
C << monoid.max(A | B)
"""
if require_monoid is not None:
warnings.warn(
"require_monoid keyword is deprecated; "
"future behavior will be like `require_monoid=False`",
DeprecationWarning,
)
else:
require_monoid = False
method_name = "ewise_add"
other = self._expect_type(
other,
Expand All @@ -1736,17 +1727,7 @@ def ewise_add(self, other, op=monoid.plus, *, require_monoid=None):
)
op = get_typed_op(op, self.dtype, other.dtype, kind="binary")
# Per the spec, op may be a semiring, but this is weird, so don't.
if require_monoid: # pragma: no cover (deprecated)
if op.opclass != "BinaryOp" or op.monoid is None:
self._expect_op(
op,
"Monoid",
within=method_name,
argname="op",
extra_message="A BinaryOp may be given if require_monoid keyword is False.",
)
else:
self._expect_op(op, ("BinaryOp", "Monoid"), within=method_name, argname="op")
self._expect_op(op, ("BinaryOp", "Monoid"), within=method_name, argname="op")
if other.ndim == 1:
# Broadcast rowwise from the right
if self._ncols != other._size:
Expand Down
13 changes: 1 addition & 12 deletions graphblas/core/operator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,14 +318,11 @@ class TypedBuiltinBinaryOp(TypedOpBase):
__slots__ = ()
opclass = "BinaryOp"

def __call__(
self, left, right=None, *, require_monoid=None, left_default=None, right_default=None
):
def __call__(self, left, right=None, *, left_default=None, right_default=None):
if left_default is not None or right_default is not None:
if (
left_default is None
or right_default is None
or require_monoid is not None
or right is not None
or not isinstance(left, InfixExprBase)
or left.method_name != "ewise_add"
Expand All @@ -338,14 +335,6 @@ def __call__(
"are Vectors or Matrices, and left_default and right_default are scalars."
)
return left.left.ewise_union(left.right, self, left_default, right_default)
if require_monoid is not None:
if right is not None:
raise TypeError(
f"Bad keyword argument `require_monoid=` when calling {self!r}.\n"
"require_monoid keyword may only be used when performing an ewise_add.\n"
f"For example: {self!r}(A | B, require_monoid=False)."
)
return _call_op(self, left, require_monoid=require_monoid)
return _call_op(self, left, right)

@property
Expand Down
23 changes: 2 additions & 21 deletions graphblas/core/vector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,7 @@ def _carg(self):
# to update to trigger a call to GraphBLAS
#########################################################

def ewise_add(self, other, op=monoid.plus, *, require_monoid=None):
def ewise_add(self, other, op=monoid.plus):
"""Perform element-wise computation on the union of sparse values, similar to how
one expects addition to work for sparse vectors.

Expand All @@ -823,7 +823,6 @@ def ewise_add(self, other, op=monoid.plus, *, require_monoid=None):
The other vector in the computation
op : :class:`~graphblas.core.operator.Monoid` or :class:`~graphblas.core.operator.BinaryOp`
Operator to use on intersecting values
require_monoid : deprecated

Returns
-------
Expand All @@ -841,31 +840,13 @@ def ewise_add(self, other, op=monoid.plus, *, require_monoid=None):
"""
from .matrix import Matrix, MatrixExpression, TransposedMatrix

if require_monoid is not None: # pragma: no cover (deprecated)
warnings.warn(
"require_monoid keyword is deprecated; "
"future behavior will be like `require_monoid=False`",
DeprecationWarning,
)
else:
require_monoid = False
method_name = "ewise_add"
other = self._expect_type(
other, (Vector, Matrix, TransposedMatrix), within=method_name, argname="other", op=op
)
op = get_typed_op(op, self.dtype, other.dtype, kind="binary")
# Per the spec, op may be a semiring, but this is weird, so don't.
if require_monoid: # pragma: no cover (deprecated)
if op.opclass != "BinaryOp" or op.monoid is None:
self._expect_op(
op,
"Monoid",
within=method_name,
argname="op",
extra_message="A BinaryOp may be given if require_monoid keyword is False.",
)
else:
self._expect_op(op, ("BinaryOp", "Monoid"), within=method_name, argname="op")
self._expect_op(op, ("BinaryOp", "Monoid"), within=method_name, argname="op")
if other.ndim == 2:
# Broadcast columnwise from the left
if other._nrows != self._size:
Expand Down
5 changes: 2 additions & 3 deletions graphblas/tests/test_infix.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,8 @@ def test_bad_ewise(s1, v1, A1, A2):
with pytest.raises(TypeError, match="not supported for FP64"):
A1 &= A1

# with pytest.raises(TypeError, match="require_monoid"):
op.minus(v1 | v1) # ok now
with pytest.raises(TypeError):
with pytest.raises(TypeError, match="unexpected keyword argument 'require_monoid'"):
op.minus(v1 & v1, require_monoid=False)
with pytest.raises(TypeError, match="Bad dtype"):
op.plus(v1 & v1, 1)
Expand Down Expand Up @@ -289,7 +288,7 @@ def test_apply_binary_bad(v1):
op.plus(v1)
with pytest.raises(TypeError, match="Bad type for keyword argument `right="):
op.plus(v1, v1)
with pytest.raises(TypeError, match="may only be used when performing an ewise_add"):
with pytest.raises(TypeError, match="unexpected keyword argument 'require_monoid'"):
op.plus(v1, 1, require_monoid=False)


Expand Down
3 changes: 0 additions & 3 deletions graphblas/tests/test_matrix.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,6 @@ def test_ewise_add(A):
[2, 0, 1, 2, 2, 2, 3, 3, 4, 4, 5, 5, 6],
[4, 3, 5, 3, 8, 5, 3, 7, 8, 3, 1, 7, 4],
)
# with pytest.raises(TypeError, match="require_monoid"):
A.ewise_add(B, binary.second) # okay now
# surprising that SECOND(x, empty) == x
C = A.ewise_add(B, binary.second).new()
Expand Down Expand Up @@ -3510,8 +3509,6 @@ def test_deprecated(A):
Vector.new(int)
with pytest.warns(DeprecationWarning):
Scalar.new(int)
with pytest.warns(DeprecationWarning):
binary.plus(A | A, require_monoid=True)
with pytest.warns(DeprecationWarning):
A.to_values()
with pytest.warns(DeprecationWarning):
Expand Down
7 changes: 3 additions & 4 deletions graphblas/tests/test_vector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,6 @@ def test_ewise_add(v):
# Binary, Monoid, and Semiring
v2 = Vector.from_coo([0, 3, 5, 6], [2, 3, 2, 1])
result = Vector.from_coo([0, 1, 3, 4, 5, 6], [2, 1, 3, 2, 2, 1])
# with pytest.raises(TypeError, match="require_monoid"):
v.ewise_add(v2, binary.minus) # okay now
w = v.ewise_add(v2, binary.max).new() # ok if the binaryop is part of a monoid
assert w.isequal(result)
Expand Down Expand Up @@ -2308,11 +2307,11 @@ def test_ewise_union_infix():
op(v | w, right_default=20)
with pytest.raises(TypeError):
op(v | w, left_default=20)
with pytest.raises(TypeError):
with pytest.raises(TypeError, match="unexpected keyword argument 'require_monoid'"):
op(v | w, left_default=10, right_default=20, require_monoid=True)
with pytest.raises(TypeError):
with pytest.raises(TypeError, match="unexpected keyword argument 'require_monoid'"):
op(v | w, left_default=10, right_default=20, require_monoid=False)
with pytest.raises(TypeError):
with pytest.raises(TypeError, match="unexpected keyword argument 'require_monoid'"):
op(v | w, right_default=20, require_monoid=True)
with pytest.raises(TypeError):
op(v & w, 5, left_default=10, right_default=20)
Expand Down
32 changes: 19 additions & 13 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,43 +175,45 @@ select = [
"W", # pycodestyle Warning
# "C90", # mccabe
# "I", # isort
# "N", # pep8-naming
"D", # pydocstyle
"UP", # pyupgrade
# "N", # pep8-naming
"YTT", # flake8-2020
# "ANN", # flake8-annotations
"S", # bandit
# "BLE", # flake8-blind-except
# "FBT", # flake8-boolean-trap
"B", # flake8-bugbear
# "A", # flake8-builtins
"COM", # flake8-commas
"C4", # flake8-comprehensions
"DTZ", # flake8-datetimez
"T10", # flake8-debugger
# "EM", # flake8-errmsg
"EXE", # flake8-executable
"ISC", # flake8-implicit-str-concat
# "ICN", # flake8-import-conventions
"G", # flake8-logging-format
# "INP", # flake8-no-pep420
"PIE", # flake8-pie
"T20", # flake8-print
"PT", # flake8-pytest-style
"Q", # flake8-quotes
# "RET", # flake8-return
"SIM", # flake8-simplify
# "TID", # flake8-tidy-imports
"TCH", # flake8-type-checking
# "ARG", # flake8-unused-arguments
"DTZ", # flake8-datetimez
# "PTH", # flake8-use-pathlib
# "ERA", # eradicate
# "PD", # pandas-vet
"PGH", # pygrep-hooks
"PL", # pylint
"PLC", # pylint Convention
"PLE", # pylint Error
"PLR", # pylint Refactor
"PLW", # pylint Warning
"PIE", # flake8-pie
"COM", # flake8-commas
# "INP", # flake8-no-pep420
"EXE", # flake8-executable
"TYP", # flake8-type-checking
"TRY", # tryceratops
# "PTH", # flake8-use-pathlib
"RUF", # ruff-specific rules
]
external = [
Expand All @@ -231,22 +233,26 @@ ignore = [
"D213", # Multi-line docstring summary should start at the second line
"D401", # First line of docstring should be in imperative mood:
"D417", # Missing argument description in the docstring:
"PLE0605", # Invalid format for `__all__`, must be `tuple` or `list` (Note: broken in v0.0.237)

# Maybe consider
"TRY004", # Prefer `TypeError` exception for invalid type (Note: good advice, but not worth the nuisance)
"TRY200", # Use `raise from` to specify exception cause (Note: sometimes okay to raise original exception)

# Intentionally ignored
"COM812", # Trailing comma missing
"D203", # 1 blank line required before class docstring (Note: conflicts with D211, which is preferred)
"SIM102", # Use a single `if` statement instead of nested `if` statements (Note: often necessary)
"SIM105", # Use contextlib.suppress(...) instead of try-except-pass (Note: try-except-pass is much faster)
"SIM108", # Use ternary operator ... instead of if-else-block (Note: if-else better for coverage and sometimes clearer)
# "SIM401", # Use dict.get ... instead of if-else-block (Note: if-else better for coverage and sometimes clearer)
"PLR2004", # Magic number used in comparison, consider replacing magic with a constant variable
"COM812", # Trailing comma missing
"PT001", # Use `@pytest.fixture()` over `@pytest.fixture` (Note: why?)
"PT003", # `scope='function'` is implied in `@pytest.fixture()` (Note: no harm in being explicit)
"PT023", # Use `@pytest.mark.slow()` over `@pytest.mark.slow` (Note: why?)
"S110", # `try`-`except`-`pass` detected, consider logging the exception (Note: good advice, but we don't log)
"SIM102", # Use a single `if` statement instead of nested `if` statements (Note: often necessary)
"SIM105", # Use contextlib.suppress(...) instead of try-except-pass (Note: try-except-pass is much faster)
"SIM108", # Use ternary operator ... instead of if-else-block (Note: if-else better for coverage and sometimes clearer)
"SIM300", # Yoda conditions are discouraged, use ... instead (Note: we're not this picky)
# "SIM401", # Use dict.get ... instead of if-else-block (Note: if-else better for coverage and sometimes clearer)
"TRY003", # Avoid specifying long messages outside the exception class (Note: why?)
]
[tool.ruff.per-file-ignores]
"graphblas/core/operator.py" = ["S102"]
Expand Down