Repository: python-graphblas/python-graphblas

Issue:
Making aggregators callable
We punted on the option of making aggregators callable.  Let's revisit.

I think this is natural: `agg.max(A)` -> `Scalar`.  So, I think aggregators should be callable to reduce to scalar.

I think common practice should be/will be to reference aggregators as `agg.sum(v)`--i.e., from `agg` namespace.  This gives adequate context to keep it readable.

I think the big question is whether we should have a way to reduce rowwise or columnwise, such as:
- `agg.max(A, rowwise=True)`
- `agg.max.rowwise(A)`

I think keyword arguments are more discoverable, and other spellings exist.

Discussion:
coveralls: 
[![Coverage Status](https://coveralls.io/builds/49343807/badge)](https://coveralls.io/builds/49343807)

Coverage decreased (-0.007%) to 99.065% when pulling **e9e2ccb347eb5c6463eb29c29f325456e672e889 on eriknw:agg_callable** into **8a6a4c52307535ea638edf87f752468e9a8a57d1 on python-graphblas:main**.


Patch:
diff --git a/graphblas/_agg.py b/graphblas/_agg.py
index eb43c2a..e9dad98 100644
--- a/graphblas/_agg.py
+++ b/graphblas/_agg.py
@@ -5,6 +5,7 @@ import numpy as np
 
 from . import agg, binary, monoid, semiring, unary
 from .dtypes import INT64, lookup_dtype
+from .utils import output_type
 
 
 def _get_types(ops, initdtype):
@@ -99,6 +100,36 @@ class Aggregator:
     def __reduce__(self):
         return f"agg.{self.name}"
 
+    def __call__(self, val, *, rowwise=False, columnwise=False):
+        # Should we expose `allow_empty=` keyword when reducing to Scalar?
+        from .matrix import Matrix, TransposedMatrix
+        from .vector import Vector
+
+        typ = output_type(val)
+        if typ is Vector:
+            if rowwise or columnwise:
+                raise ValueError(
+                    "rowwise and columnwise arguments should not be used with Vector input"
+                )
+            return val.reduce(self)
+        elif typ in {Matrix, TransposedMatrix}:
+            if rowwise:
+                if columnwise:
+                    raise ValueError("rowwise and columnwise arguments cannot both be True")
+                return val.reduce_rowwise(self)
+            elif columnwise:
+                return val.reduce_columnwise(self)
+            else:
+                return val.reduce_scalar(self)
+        else:
+            raise TypeError(
+                f"Bad type when calling {self!r}.\n"
+                "    - Expected type: Vector, Matrix, TransposedMatrix.\n"
+                f"    - Got: {type(val)}.\n"
+                "Calling an Aggregator is syntactic sugar for calling reduce methods.  "
+                f"For example, `A.reduce_scalar({self!r})` is the same as `{self!r}(A)`."
+            )
+
 
 class TypedAggregator:
     opclass = "Aggregator"
@@ -238,6 +269,8 @@ class TypedAggregator:
     def __reduce__(self):
         return (getitem, (self.parent, self.type))
 
+    __call__ = Aggregator.__call__
+
 
 # Monoid-only
 agg.sum = Aggregator("sum", monoid=monoid.plus)
diff --git a/graphblas/_automethods.py b/graphblas/_automethods.py
index de4fd8e..0e374fc 100644
--- a/graphblas/_automethods.py
+++ b/graphblas/_automethods.py
@@ -174,6 +174,10 @@ def gb_obj(self):
     return self._get_value("gb_obj")
 
 
+def get(self):
+    return self._get_value("get")
+
+
 def inner(self):
     return self._get_value("inner")
 
@@ -316,6 +320,7 @@ def _main():
         "_name_html",
         "_nvals",
         "gb_obj",
+        "get",
         "isclose",
         "isequal",
         "name",
diff --git a/graphblas/infix.py b/graphblas/infix.py
index 1c1003a..746b25c 100644
--- a/graphblas/infix.py
+++ b/graphblas/infix.py
@@ -81,6 +81,7 @@ class VectorInfixExpr(InfixExprBase):
     ewise_mult = wrapdoc(Vector.ewise_mult)(property(_automethods.ewise_mult))
     ewise_union = wrapdoc(Vector.ewise_union)(property(_automethods.ewise_union))
     gb_obj = wrapdoc(Vector.gb_obj)(property(_automethods.gb_obj))
+    get = wrapdoc(Vector.get)(property(_automethods.get))
     inner = wrapdoc(Vector.inner)(property(_automethods.inner))
     isclose = wrapdoc(Vector.isclose)(property(_automethods.isclose))
     isequal = wrapdoc(Vector.isequal)(property(_automethods.isequal))
@@ -196,6 +197,7 @@ class MatrixInfixExpr(InfixExprBase):
     ewise_mult = wrapdoc(Matrix.ewise_mult)(property(_automethods.ewise_mult))
     ewise_union = wrapdoc(Matrix.ewise_union)(property(_automethods.ewise_union))
     gb_obj = wrapdoc(Matrix.gb_obj)(property(_automethods.gb_obj))
+    get = wrapdoc(Matrix.get)(property(_automethods.get))
     isclose = wrapdoc(Matrix.isclose)(property(_automethods.isclose))
     isequal = wrapdoc(Matrix.isequal)(property(_automethods.isequal))
     kronecker = wrapdoc(Matrix.kronecker)(property(_automethods.kronecker))
@@ -312,6 +314,7 @@ class ScalarMatMulExpr(InfixExprBase):
     _name_html = wrapdoc(Scalar._name_html)(property(_automethods._name_html))
     _nvals = wrapdoc(Scalar._nvals)(property(_automethods._nvals))
     gb_obj = wrapdoc(Scalar.gb_obj)(property(_automethods.gb_obj))
+    get = wrapdoc(Scalar.get)(property(_automethods.get))
     is_empty = wrapdoc(Scalar.is_empty)(property(_automethods.is_empty))
     isclose = wrapdoc(Scalar.isclose)(property(_automethods.isclose))
     isequal = wrapdoc(Scalar.isequal)(property(_automethods.isequal))
diff --git a/graphblas/matrix.py b/graphblas/matrix.py
index daeb0c4..95a8fe1 100644
--- a/graphblas/matrix.py
+++ b/graphblas/matrix.py
@@ -393,6 +393,20 @@ class Matrix(BaseType):
         # TODO: expose COMPLETE or MATERIALIZE options to the user
         call("GrB_Matrix_wait", [self, _MATERIALIZE])
 
+    def get(self, row, col, default=None):
+        """Get an element at row, col indices as a Python scalar.
+
+        If there is no element at ``matrix[row, col]``, then the default value is returned.
+        """
+        expr = self[row, col]
+        if expr._is_scalar:
+            rv = expr.new().value
+            return default if rv is None else rv
+        raise ValueError(
+            "Bad row, col arguments in Matrix.get(...).  "
+            "Indices should get a single element, which will be extracted as a Python scalar."
+        )
+
     @classmethod
     def new(cls, dtype, nrows=0, ncols=0, *, name=None):
         """
@@ -1604,6 +1618,7 @@ class MatrixExpression(BaseExpression):
     ewise_mult = wrapdoc(Matrix.ewise_mult)(property(_automethods.ewise_mult))
     ewise_union = wrapdoc(Matrix.ewise_union)(property(_automethods.ewise_union))
     gb_obj = wrapdoc(Matrix.gb_obj)(property(_automethods.gb_obj))
+    get = wrapdoc(Matrix.get)(property(_automethods.get))
     isclose = wrapdoc(Matrix.isclose)(property(_automethods.isclose))
     isequal = wrapdoc(Matrix.isequal)(property(_automethods.isequal))
     kronecker = wrapdoc(Matrix.kronecker)(property(_automethods.kronecker))
@@ -1684,6 +1699,7 @@ class MatrixIndexExpr(AmbiguousAssignOrExtract):
     ewise_mult = wrapdoc(Matrix.ewise_mult)(property(_automethods.ewise_mult))
     ewise_union = wrapdoc(Matrix.ewise_union)(property(_automethods.ewise_union))
     gb_obj = wrapdoc(Matrix.gb_obj)(property(_automethods.gb_obj))
+    get = wrapdoc(Matrix.get)(property(_automethods.get))
     isclose = wrapdoc(Matrix.isclose)(property(_automethods.isclose))
     isequal = wrapdoc(Matrix.isequal)(property(_automethods.isequal))
     kronecker = wrapdoc(Matrix.kronecker)(property(_automethods.kronecker))
@@ -1827,6 +1843,7 @@ class TransposedMatrix:
     __ixor__ = _automethods.__ixor__
 
     # Misc.
+    get = Matrix.get
     isequal = Matrix.isequal
     isclose = Matrix.isclose
     wait = Matrix.wait
diff --git a/graphblas/scalar.py b/graphblas/scalar.py
index 6ae5e81..478d5e4 100644
--- a/graphblas/scalar.py
+++ b/graphblas/scalar.py
@@ -369,6 +369,10 @@ class Scalar(BaseType):
         if not self._is_cscalar:
             call("GrB_Scalar_wait", [self, _MATERIALIZE])
 
+    def get(self, default=None):
+        """Get the value of a scalar as a Python scalar or the default value if it is empty."""
+        return default if self._is_empty else self.value
+
     @classmethod
     def new(cls, dtype, *, is_cscalar=False, name=None):
         """
@@ -544,6 +548,7 @@ class ScalarExpression(BaseExpression):
     _name_html = wrapdoc(Scalar._name_html)(property(_automethods._name_html))
     _nvals = wrapdoc(Scalar._nvals)(property(_automethods._nvals))
     gb_obj = wrapdoc(Scalar.gb_obj)(property(_automethods.gb_obj))
+    get = wrapdoc(Scalar.get)(property(_automethods.get))
     is_empty = wrapdoc(Scalar.is_empty)(property(_automethods.is_empty))
     isclose = wrapdoc(Scalar.isclose)(property(_automethods.isclose))
     isequal = wrapdoc(Scalar.isequal)(property(_automethods.isequal))
@@ -604,6 +609,7 @@ class ScalarIndexExpr(AmbiguousAssignOrExtract):
     _name_html = wrapdoc(Scalar._name_html)(property(_automethods._name_html))
     _nvals = wrapdoc(Scalar._nvals)(property(_automethods._nvals))
     gb_obj = wrapdoc(Scalar.gb_obj)(property(_automethods.gb_obj))
+    get = wrapdoc(Scalar.get)(property(_automethods.get))
     is_empty = wrapdoc(Scalar.is_empty)(property(_automethods.is_empty))
     isclose = wrapdoc(Scalar.isclose)(property(_automethods.isclose))
     isequal = wrapdoc(Scalar.isequal)(property(_automethods.isequal))
diff --git a/graphblas/unary/numpy.py b/graphblas/unary/numpy.py
index f2c0097..5b0e546 100644
--- a/graphblas/unary/numpy.py
+++ b/graphblas/unary/numpy.py
@@ -77,7 +77,7 @@ _numpy_to_graphblas = {
     "arctan": "atan",
     "arctanh": "atanh",
     "bitwise_not": "bnot",
-    # "cbrt": "cbrt",  # TODO: added in SuiteSparse:GraphBLAS 7.1.0
+    "cbrt": "cbrt",
     "ceil": "ceil",
     "cos": "cos",
     "cosh": "cosh",
diff --git a/graphblas/vector.py b/graphblas/vector.py
index 53a18a2..fdfde6f 100644
--- a/graphblas/vector.py
+++ b/graphblas/vector.py
@@ -380,6 +380,20 @@ class Vector(BaseType):
         # TODO: expose COMPLETE or MATERIALIZE options to the user
         call("GrB_Vector_wait", [self, _MATERIALIZE])
 
+    def get(self, index, default=None):
+        """Get an element at index as a Python scalar.
+
+        If there is no element at ``vector[index]``, then the default value is returned.
+        """
+        expr = self[index]
+        if expr._is_scalar:
+            rv = expr.new().value
+            return default if rv is None else rv
+        raise ValueError(
+            "Bad index in Vector.get(...).  "
+            "A single index should be given, and the result will be a Python scalar."
+        )
+
     @classmethod
     def new(cls, dtype, size=0, *, name=None):
         """
@@ -1220,6 +1234,7 @@ class VectorExpression(BaseExpression):
     ewise_mult = wrapdoc(Vector.ewise_mult)(property(_automethods.ewise_mult))
     ewise_union = wrapdoc(Vector.ewise_union)(property(_automethods.ewise_union))
     gb_obj = wrapdoc(Vector.gb_obj)(property(_automethods.gb_obj))
+    get = wrapdoc(Vector.get)(property(_automethods.get))
     inner = wrapdoc(Vector.inner)(property(_automethods.inner))
     isclose = wrapdoc(Vector.isclose)(property(_automethods.isclose))
     isequal = wrapdoc(Vector.isequal)(property(_automethods.isequal))
@@ -1292,6 +1307,7 @@ class VectorIndexExpr(AmbiguousAssignOrExtract):
     ewise_mult = wrapdoc(Vector.ewise_mult)(property(_automethods.ewise_mult))
     ewise_union = wrapdoc(Vector.ewise_union)(property(_automethods.ewise_union))
     gb_obj = wrapdoc(Vector.gb_obj)(property(_automethods.gb_obj))
+    get = wrapdoc(Vector.get)(property(_automethods.get))
     inner = wrapdoc(Vector.inner)(property(_automethods.inner))
     isclose = wrapdoc(Vector.isclose)(property(_automethods.isclose))
     isequal = wrapdoc(Vector.isequal)(property(_automethods.isequal))


Test patch:
diff --git a/graphblas/tests/test_matrix.py b/graphblas/tests/test_matrix.py
index 09eeeef..0476f6e 100644
--- a/graphblas/tests/test_matrix.py
+++ b/graphblas/tests/test_matrix.py
@@ -1498,6 +1498,23 @@ def test_reduce_scalar(A):
     assert t == 48.23
 
 
+@autocompute
+def test_reduce_call_agg(A):
+    assert agg.sum(A) == 47
+    assert agg.sum(A.T) == 47
+    assert agg.sum(A + A) == 94  # handles expression
+    result = agg.max[float](A).new()  # typed agg is callable too
+    assert result.dtype == "FP64"
+    assert result == 8
+    expected = Vector.from_values([0, 1, 2, 3, 4, 5, 6], [5, 12, 1, 6, 7, 1, 15])
+    result = agg.sum(A, rowwise=True)
+    assert result.isequal(expected)
+    result = agg.sum(A.T, columnwise=True)
+    assert result.isequal(expected)
+    with pytest.raises(ValueError, match="cannot both be True"):
+        agg.sum(A, rowwise=True, columnwise=True)
+
+
 def test_transpose(A):
     # C << A.T
     rows, cols, vals = A.to_values()
@@ -3438,3 +3455,16 @@ def test_to_values_sort():
     assert_array_equal(rows, expected_rows)
     rows, cols, values = A.T.to_values(sort=True)
     assert_array_equal(cols, expected_rows)
+
+
+def test_get(A):
+    assert compute(A.get(0, 0)) is None
+    assert A.get(0, 0, "mittens") == "mittens"
+    assert A.get(0, 1) == 2
+    assert compute(A.T.get(0, 1)) is None
+    assert A.T.get(1, 0) == 2
+    assert A.get(0, 1, "mittens") == 2
+    assert type(compute(A.get(0, 1))) is int
+    with pytest.raises(ValueError):
+        # Not yet supported
+        A.get(0, [0, 1])
diff --git a/graphblas/tests/test_scalar.py b/graphblas/tests/test_scalar.py
index 6b659fc..8b0b21e 100644
--- a/graphblas/tests/test_scalar.py
+++ b/graphblas/tests/test_scalar.py
@@ -449,3 +449,12 @@ def test_record_from_dict():
         {"x": 1, "y": {"a": 2, "b": 3}}, dtype={"x": int, "y": {"a": int, "b": int}}
     )
     assert s == (1, (2, 3))
+
+
+def test_get(s):
+    assert s.get() == 5
+    assert s.get("mittens") == 5
+    assert type(compute(s.get())) is int
+    s.clear()
+    assert compute(s.get()) is None
+    assert s.get("mittens") == "mittens"
diff --git a/graphblas/tests/test_vector.py b/graphblas/tests/test_vector.py
index 1ebb664..af805db 100644
--- a/graphblas/tests/test_vector.py
+++ b/graphblas/tests/test_vector.py
@@ -876,6 +876,21 @@ def test_reduce_coerce_dtype(v):
     assert t == 5.23
 
 
+@autocompute
+def test_reduce_call_agg(v):
+    assert agg.sum(v) == 4
+    assert agg.sum(v + v) == 8  # handles expression
+    result = agg.max[float](v).new()  # typed agg is callable too
+    assert result.dtype == "FP64"
+    assert result == 2
+    with pytest.raises(ValueError, match="rowwise"):
+        agg.sum(v, rowwise=True)
+    with pytest.raises(ValueError, match="columnwise"):
+        agg.sum(v, columnwise=True)
+    with pytest.raises(TypeError):
+        agg.sum(7)
+
+
 def test_simple_assignment(v):
     # w[:] = v
     w = Vector(v.dtype, v.size)
@@ -2081,3 +2096,14 @@ def test_lambda_udfs(v):
     v.ewise_add(v, lambda x, y: x + y)  # now okay
     with pytest.raises(TypeError):
         v.inner(v, lambda x, y: x + y)
+
+
+def test_get(v):
+    assert compute(v.get(0)) is None
+    assert v.get(0, "mittens") == "mittens"
+    assert v.get(1) == 1
+    assert v.get(1, "mittens") == 1
+    assert type(compute(v.get(1))) is int
+    with pytest.raises(ValueError):
+        # Not yet supported
+        v.get([0, 1])