From 2083cceb39ac88a7e3b18111b06db205019a6369 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Wed, 25 Jan 2023 13:41:49 -0600 Subject: [PATCH 1/4] Remove deprecated `require_monoid` This was deprecated on 2022-05-23, so we could have actually removed it in 2023.1.0. --- graphblas/core/matrix.py | 23 ++--------------------- graphblas/core/operator.py | 13 +------------ graphblas/core/vector.py | 23 ++--------------------- graphblas/tests/test_infix.py | 5 ++--- graphblas/tests/test_matrix.py | 3 --- graphblas/tests/test_vector.py | 7 +++---- 6 files changed, 10 insertions(+), 64 deletions(-) diff --git a/graphblas/core/matrix.py b/graphblas/core/matrix.py index 3cd0b6c7f..9dbfd2320 100644 --- a/graphblas/core/matrix.py +++ b/graphblas/core/matrix.py @@ -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. @@ -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 ------- @@ -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, @@ -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: diff --git a/graphblas/core/operator.py b/graphblas/core/operator.py index c5e518c99..6a726cc0c 100644 --- a/graphblas/core/operator.py +++ b/graphblas/core/operator.py @@ -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" @@ -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 diff --git a/graphblas/core/vector.py b/graphblas/core/vector.py index 47a6a1e6a..e0d55cc99 100644 --- a/graphblas/core/vector.py +++ b/graphblas/core/vector.py @@ -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. @@ -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 ------- @@ -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: diff --git a/graphblas/tests/test_infix.py b/graphblas/tests/test_infix.py index 3d6a674b2..f496ade15 100644 --- a/graphblas/tests/test_infix.py +++ b/graphblas/tests/test_infix.py @@ -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) @@ -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) diff --git a/graphblas/tests/test_matrix.py b/graphblas/tests/test_matrix.py index 68d7a6f9b..57bf6e5e3 100644 --- a/graphblas/tests/test_matrix.py +++ b/graphblas/tests/test_matrix.py @@ -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() @@ -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): diff --git a/graphblas/tests/test_vector.py b/graphblas/tests/test_vector.py index dcd746369..c7f90c10c 100644 --- a/graphblas/tests/test_vector.py +++ b/graphblas/tests/test_vector.py @@ -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) @@ -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) From 37249b045a886b318f1ffc10252f1e6e7eb85d5e Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Thu, 26 Jan 2023 13:49:44 -0600 Subject: [PATCH 2/4] alphabetical ruff --- .pre-commit-config.yaml | 4 ++-- pyproject.toml | 28 +++++++++++++++------------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d2bd720cd..949fc7b53 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -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 hooks: - id: validate-pyproject name: Validate pyproject.toml @@ -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.235 hooks: - id: ruff - repo: https://github.com/sphinx-contrib/sphinx-lint diff --git a/pyproject.toml b/pyproject.toml index ff117c350..528f101c8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -175,9 +175,9 @@ 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 @@ -185,33 +185,35 @@ select = [ # "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 = [ @@ -237,16 +239,16 @@ ignore = [ "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?) + "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) ] [tool.ruff.per-file-ignores] "graphblas/core/operator.py" = ["S102"] From 424febf674739e747d6411b04d096fe730d58562 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Fri, 27 Jan 2023 17:57:02 -0600 Subject: [PATCH 3/4] bump --- .pre-commit-config.yaml | 4 ++-- pyproject.toml | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 949fc7b53..33050f19e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -20,7 +20,7 @@ repos: - id: mixed-line-ending - id: trailing-whitespace - repo: https://github.com/abravalheri/validate-pyproject - rev: v0.12 + rev: v0.12.1 hooks: - id: validate-pyproject name: Validate pyproject.toml @@ -71,7 +71,7 @@ repos: additional_dependencies: [tomli] files: ^(graphblas|docs)/ - repo: https://github.com/charliermarsh/ruff-pre-commit - rev: v0.0.235 + rev: v0.0.236 hooks: - id: ruff - repo: https://github.com/sphinx-contrib/sphinx-lint diff --git a/pyproject.toml b/pyproject.toml index 528f101c8..c89ab12de 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -193,7 +193,7 @@ select = [ "EXE", # flake8-executable "ISC", # flake8-implicit-str-concat # "ICN", # flake8-import-conventions - # "G", # flake8-logging-format + "G", # flake8-logging-format # "INP", # flake8-no-pep420 "PIE", # flake8-pie "T20", # flake8-print @@ -249,6 +249,7 @@ ignore = [ "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) + "TRY003", # Avoid specifying long messages outside the exception class (Note: why?) ] [tool.ruff.per-file-ignores] "graphblas/core/operator.py" = ["S102"] From 9fd714975d0ebeadaca14c7da180d555728d0bbb Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Sat, 28 Jan 2023 14:55:36 -0600 Subject: [PATCH 4/4] bump --- .pre-commit-config.yaml | 4 ++-- pyproject.toml | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 33050f19e..dc514793f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -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 @@ -71,7 +71,7 @@ repos: additional_dependencies: [tomli] files: ^(graphblas|docs)/ - repo: https://github.com/charliermarsh/ruff-pre-commit - rev: v0.0.236 + rev: v0.0.237 hooks: - id: ruff - repo: https://github.com/sphinx-contrib/sphinx-lint diff --git a/pyproject.toml b/pyproject.toml index c89ab12de..d88f1bdc8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -233,6 +233,7 @@ 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) @@ -245,9 +246,11 @@ ignore = [ "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?) ]