Skip to content

Add indexunary helpers, break per-call agg import, document from_string - #604

Open
eriknw wants to merge 3 commits into
23-operator-namespace-uxfrom
24-operator-resolution
Open

Add indexunary helpers, break per-call agg import, document from_string#604
eriknw wants to merge 3 commits into
23-operator-namespace-uxfrom
24-operator-resolution

Conversation

@eriknw

@eriknw eriknw commented Aug 4, 2026

Copy link
Copy Markdown
Member

Three improvements to operator resolution and its API surface.

  • Break the per-call agg import in get_typed_op (51e88bc): agg.py
    registers its Aggregator and TypedAggregator classes with
    operator/utils.py when it loads, so get_typed_op consults a registry
    instead of running an import statement (a repeated sys.modules lookup)
    on every operator resolution.
  • Add indexunary.value/row/column helpers accepting comparison
    expressions
    (f5451bf): indexunary.value(A > 3) mirrors select's
    helpers, with the shared resolution logic moved to core/operator/utils.py
    and select delegating to it unchanged. indexunary.index is deliberately
    not added: it already exists as a working operator alias for rowindex,
    and shadowing it is a maintainer call. Closes three quarters of indexunary is missing helpers value, row, column, and index #239.
  • Add docstrings to the operator from_string helpers (adc87bf): all
    eight namespaces' from_string helpers were undocumented; each now has a
    numpydoc docstring with runnable examples covering names, dotted paths,
    shorthand symbols, and the [dtype] suffix. Also fixes a stale example in
    the indexunary from_string error message ('row_index' does not resolve;
    the operator is 'rowindex'). GH <op>.from_string functions need docstrings #513.

Stack created with GitHub Stacks CLIGive Feedback 💬

@eriknw eriknw changed the title 24 operator resolution Add indexunary helpers, break per-call agg import, document from_string Aug 4, 2026
@eriknw
eriknw marked this pull request as ready for review August 4, 2026 16:07
@eriknw
eriknw force-pushed the 24-operator-resolution branch from adc87bf to d4117f9 Compare August 4, 2026 16:12
@eriknw
eriknw force-pushed the 24-operator-resolution branch 4 times, most recently from a4829ec to 5380d4f Compare August 5, 2026 18:03
@eriknw
eriknw force-pushed the 24-operator-resolution branch from 5380d4f to 90cb7d9 Compare August 5, 2026 18:05
@eriknw
eriknw force-pushed the 24-operator-resolution branch from 90cb7d9 to 7ac6414 Compare August 6, 2026 07:59
@eriknw
eriknw force-pushed the 24-operator-resolution branch from 7ac6414 to 44f7d06 Compare August 6, 2026 15:39
@eriknw
eriknw force-pushed the 24-operator-resolution branch from 44f7d06 to 89b2cf8 Compare August 6, 2026 15:41
@eriknw
eriknw force-pushed the 24-operator-resolution branch from 89b2cf8 to 89235b6 Compare August 6, 2026 20:36
@eriknw
eriknw force-pushed the 24-operator-resolution branch from 89235b6 to 397d63f Compare August 6, 2026 20:42
@eriknw
eriknw force-pushed the 24-operator-resolution branch from 397d63f to f7241d9 Compare August 7, 2026 02:48
eriknw added 3 commits August 7, 2026 00:09
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'.
@eriknw
eriknw force-pushed the 24-operator-resolution branch from f7241d9 to 7611a08 Compare August 7, 2026 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant