Skip to content

Commit

Permalink
Remove deprecated require_monoid (python-graphblas#377)
Browse files Browse the repository at this point in the history
* Remove deprecated `require_monoid`

This was deprecated on 2022-05-23, so we could have actually removed it in 2023.1.0.

* alphabetical ruff
  • Loading branch information
eriknw authored Jan 28, 2023
1 parent e34b6dd commit e7a9a31
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 80 deletions.
6 changes: 3 additions & 3 deletions .pre-commit-config.yaml
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
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
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
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
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
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
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
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

0 comments on commit e7a9a31

Please sign in to comment.