GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |
GeezerVison is showing github.com — Add indexunary helpers, break per-call agg import, document from_string by eriknw · Pull Request #604 · python-graphblas/python-graphblas Conversation
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@ ## 49-masked-literal-assign #604 +/- ## ============================================================ + Coverage 98.62% 98.86% +0.23% ============================================================ Files 106 106 Lines 33006 33142 +136 Branches 3837 3859 +22 ============================================================ + Hits 32553 32766 +213 + Misses 332 234 -98 - Partials 121 142 +21 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
get_typed_op imported the agg module inside the function on every operator resolution, to isinstance-check Aggregator and TypedAggregator. The import had to be function-local because agg.py imports get_typed_op, so a module-level import would be a cycle. agg.py now registers its two classes with operator/utils.py when it loads, and get_typed_op consults the registry instead of running an import statement per call. The None guard is safe because those classes are the only way to make an Aggregator: before agg.py finishes loading no instance can exist, and agg.py's own body never hands one to get_typed_op (its two call sites pass a monoid and a semiring, both from inside functions rather than at module level). This does not change when the agg module loads. core/operator/__init__.py imports Aggregator eagerly, so agg is already in sys.modules before any user code can reach get_typed_op; the function-local import was repeating a sys.modules lookup rather than deferring a module load.
…ons (gh-239) gh-239 asks for value, row, column and index helpers in the indexunary namespace, mirroring the four that select already has. This adds three of them: indexunary.value(A > 3) returns the same expression as A.apply(indexunary.valuegt, 3), with the same input validation, the same lt-to-le and ge-to-gt thunk shifts, and the same errors as select's helpers, adapted to name indexunary. The shared resolution logic moves to core/operator/utils.py; select delegates to it and is unchanged. index is deliberately not added, so this closes three quarters of gh-239. indexunary.index already exists as an operator alias for rowindex, and is used as one (v.apply(indexunary.index) and indexunary.index(v) are both tested), so a helper function of that name would shadow a working operator. Adding it needs a decision about that alias, which is a maintainer call. Vector index comparisons are meanwhile reachable through indexunary.row(v < k) or the indexle and indexgt aliases. New matrix and vector tests compare each helper against the explicit apply, and cover the BOOL result dtype, the thunk shifts, the scalar expression path, and the error cases.
All eight operator-namespace from_string helpers (unary, binary, monoid, semiring, select, indexunary, agg, op) had no docstring at all. Add numpydoc docstrings describing the accepted string forms: a namespace name, a dotted path such as numpy.mod, shorthand symbols such as + or >=, and the [dtype] typing suffix. Each carries runnable examples. test_from_string gains select and indexunary cases and asserts every namespace's from_string is now documented. Also fix a stale example in the indexunary from_string error message. It offered 'row_index', which does not resolve; the operator is 'rowindex'.
agg.py registers the Aggregator classes when core.operator loads it, at import, and no call reaches this check before then, so the arm where _Aggregator is still None never runs (the guard is there in case one ever does).
…by name
_from_string took any attribute of the namespace module, so the new
helper functions, and private names and methods, came back as operators:
indexunary.from_string("value") (also "row", "column", "VALUE")
before: <function value> ("value[int]": TypeError, not subscriptable)
after: ValueError: Unknown indexunary string: 'value'
as on the parent, and as the from_string docstrings say (IndexUnaryOp).
select.from_string("value") and op.from_string("register_new") (a bound
method) are refused too. Applying a helper reached Numba as a UDF:
A.apply(indexunary.value)
before: UdfParseError: unary.value does not work with (INT64): Untyped
global name '_resolve_expr': ...
after: TypeError: indexunary.value is a helper, not an operator: it
takes a comparison and returns its expression, as in
indexunary.value(A > 3). To apply an op, give one such as
indexunary.valuegt.
_resolve_index_expr took any expression, so a wrong one failed somewhere
inside, or reached the op it picked, and a Scalar's comparison crashed in
the < and >= shift, which read the thunk before taking it out of its Vector:
select.row(s < 1), indexunary.row(s >= 0.5) (s a Scalar)
before: AttributeError: 'Vector' object has no attribute 'value'
after: s, and False
indexunary.value(abs(A)) before: IndexError: list index out of range
indexunary.value(A.mxm(A)) before: AttributeError: 'Matrix' object ...
indexunary.value(A.apply(gt, left=3)) before: TypeError: Do not pass `left` ...
indexunary.value(A.apply(plus, 3)) before: ValueError: Unknown or unregistered
indexunary method: valueplus
after: TypeError: indexunary.value takes a comparison of a Vector, Matrix or
Scalar with a scalar on its right, as in indexunary.value(x <= 5);
got M_0.apply(unary.abs[INT64])
indexunary.row(A == 3)
before: ValueError: Unknown or unregistered indexunary method: roweq
after: ValueError: indexunary.row compares indices with <, <=, > or >=, not ==
indexunary.row(A < Scalar(float)) (an empty thunk)
before: TypeError: unsupported operand type(s) for -=: 'NoneType' and 'int'
after: EmptyObject, as A.apply(indexunary.rowle, Scalar(float)) raises
_check_index_expr accepts a built-in comparison applied with a scalar on
the right (a Scalar's is an ewise_mult of two Vectors), and for row and
column only the four orderings. The shift now runs after the Scalar unwrap
and leaves an empty thunk to the op. With these checks the "Unknown or
unregistered" raise is unreachable again, so its pragma stands. Select's
helpers share the function and change the same way; main's select had
the same crashes.
…ss does
_resolve_index_expr looked the op up in the namespace's globals, which
hold an op registered with lazy=True only once something touches it:
indexunary.register_new("rowlt", lambda x, i, j, t: i < t, lazy=True)
indexunary.row(A < 2.5) (rows 0 to 3)
before: [True, True, False, False] (the <= shift), then [True, True,
True, False] once indexunary.rowlt was touched
after: [True, True, True, False] both times, from the op
_namespace_op builds such an op through the module's __getattr__, as
indexunary.rowlt would. select.row does the same; main's select did too.
The comment said no Aggregator can exist before agg.py registers its classes, but agg.py builds every built-in Aggregator at module level first. What holds is that none reaches get_typed_op until then.
"Add docstrings to the operator from_string helpers (GH #513)" fixed the example in that message (row_index, which does not resolve, became rowindex) without a test; on its parent this one fails.
select.index(v < 4) filters by index, but indexunary.index is the rowindex op (a helper would shadow it), so indexunary.index(v < 4) gives each entry's index without an error. The note for that was a code comment; the row helper's docstring now says to use row for a Vector.
Port of PR A's "Compare indices with the bound given in select.row, column and index" to _resolve_index_expr, which the select and indexunary helpers share here. When #604's chain is replayed over A, its "Add indexunary.value/row/column helpers accepting comparison expressions (gh-239)" conflicts in graphblas/select/__init__.py; take that commit's own version of the file (select delegates here), and this commit carries A's fix into the shared function. The helpers read the thunk from the comparison x < k, which types k beside x's values: beside INT8 or UINT8, 300 is infinity, and beside FP32, 2**24 + 1 rounds. indexunary.row(B <= 299) on a 400-row INT8 matrix was True in all 400 rows, as select.row(B < 300) kept them. Now they compare indices with the number the caller gave (the expression's _right_literal, from A), or the thunk Scalar's value, made a whole number: i < t is i <= ceil(t) - 1, i >= t is i > ceil(t) - 1, and i <= t and i > t take floor(t). An op registered for < or >= (a user's rowlt) gets the number as given. An empty Scalar thunk still reaches the op, which raises EmptyObject. Also r604's R1: a Scalar's .apply keeps its thunk a Scalar, so taking it out of a Vector crashed: select.value(s.apply(binary.lt, right=10)) raised AttributeError on suitesparse and TypeError on vanilla. The thunk is now taken out of a Vector only when it is one. Tests: indexunary.row and column on Matrix (INT8, UINT8, FP32, transposed, fractional bounds), Vector and Scalar, and the R1 case.
The same change as PR A's "Take the thunk's value as an index bound that is not a number", in _index_bound here, which the select and indexunary helpers share: indexunary.row(A < np.array(3)) kept rows 0 to 3.
An IndexUnaryOp registered with lazy=True waits in indexunary._delayed, and its SelectOp is made only when it is built. select's helpers looked in select's namespace alone, so select.row(A < 2.5) took the <= shift until something touched indexunary.rowlt, and then the op (r604 R3). _namespace_op now builds the indexunary op for select when select has none by that name, as attribute access on indexunary would. The test of a lazily registered op could not fail since the helpers compare indices with the bound given: its rowlt, i < t, answered as the <= shift does, so it passed with the lazy lookup removed (r604 P1). Its rowlt is now i == 0, which the shift does not give, and select.row runs first. It fails with either the lazy lookup or this change taken out.
… here too The same change as on #661, in _index_bound's twin in core/operator/utils.py, which the select and indexunary helpers share here: A of 5 rows: indexunary.row(A < nan), (A <= nan) before: True for row 0 only indexunary.row(A > nan), (A >= nan) before: True for rows 1 to 4 after: False for every row, and select.row keeps none (main kept row 0, or rows 1 to 4) _index_bound also reads a value only from a Scalar thunk now, as on #661, where that restored the op's TypeError for A.apply(binary.le, left=3). Here _check_index_expr refuses that form first, so the two copies stay alike without a test of it.
#661's test of select.row(A.apply(binary.le, left=3)) expects the op's TypeError, as main gives. Here the helpers refuse such an expression first, with a TypeError that names the helper ("Check the comparison given to the value, row and column helpers"), so the test now expects that message.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.