Skip to content

Resurrect mortie/batch.py as the consolidated home for the bulk operators - #172

Merged
espg merged 8 commits into
mainfrom
claude/170-batch-module
Aug 9, 2026
Merged

Resurrect mortie/batch.py as the consolidated home for the bulk operators#172
espg merged 8 commits into
mainfrom
claude/170-batch-module

Conversation

@espg

@espg espg commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Closes #170.

Resurrects mortie/batch.py as the consolidated home for the bulk operators, collecting the five plural twins that sat beside their scalars in coverage.py, moc.py, orders.py and geometry.py. The move is by arity, a different axis from the domain split of #159 / PR #169, and the mitigation the decision rests on — cross-linked scalar/plural docstrings — landed as phase 2 of this PR, not as a follow-up. All four phases are complete.

function from to
polygons_to_morton_mocs mortie/coverage.py mortie/batch.py
from_wkbs mortie/geometry.py mortie/batch.py
mocs_to_orders mortie/moc.py mortie/batch.py
common_ancestors mortie/moc.py mortie/batch.py
children_of mortie/orders.py mortie/batch.py

mortie.arrow.from_wkbs and mortie.arrow.polygons_to_morton_mocs stay in arrow.py — the pyarrow skin is #154's core/skin axis, a third one. Their bodies' function-local from .coverage import … / from .geometry import … are repointed at .batch, which is the only change to that module.

Phases

  • Phase 1 — the pure move. Five functions to batch.py, import rewiring, SPLITS extended, verifier green.
  • Phase 2 — the cross-links. Every scalar gains a See Also to its plural and every plural one back to its scalar, retargeted at the new module path. Deliberately not a pure move; retires this split's SPLITS entry (reasoning below).
  • Phase 3 — docs/api/batch.md, with the moved entries removed from their current pages' explicit members: lists, docs/api/moc.md's stale "each scalar op sits beside its plural batch twin" prose corrected, and — because mkdocstrings drops a stale members: entry silently, strict: true notwithstanding — a permanent pytest backstop asserting every members: name in docs/api/*.md resolves on its page's module.
  • Phase 4 — delete benchmarks/verify_pure_move.py (espg's ruling: "Resurrect mortie/batch.py as the consolidated home for the bulk operators, with cross-linked scalar/plural docstrings #170 should clean it up as part of its work"). Used first, deleted last — the verification below is what makes the deletion safe to do here.

Phase 1 — what landed

The moves are verbatim. Every one of the five functions is byte-identical to its pre-move original; nothing was reflowed, renamed or re-signatured. mocs_to_orders keeps max_cells=_FLAT_COVER_WARN_THRESHOLD as its default, so batch.py imports that private constant from coverage.py; from_wkbs keeps calling _wkb_bytes, imported from geometry.py. Neither source module loses an import it still needs, and batch.py is imported by nothing but __init__.py and arrow.py, so no cycle is introduced.

mortie/__init__.py's flat names do not change. __all__ is 69 names before and after, and the five are re-exported from .batch instead of from their old modules. Verified by real import rather than by reading:

polygons_to_morton_mocs      -> mortie.batch
mocs_to_orders               -> mortie.batch
common_ancestors             -> mortie.batch
children_of                  -> mortie.batch
from_wkbs                    -> mortie.batch
len(__all__) = 69

batch is deliberately not added to __all__. The only submodule names in there are geometry, morton_index and arrow; coverage, moc, orders, convert and buffer are all absent, so a new module staying out is the majority convention as well as the requirement. from mortie import * is unchanged.

Line counts, all six touched modules (origin/main → this branch):

module before after
mortie/batch.py 667
mortie/coverage.py 602 498
mortie/moc.py 644 429
mortie/orders.py 573 439
mortie/geometry.py 772 602
mortie/arrow.py 770 770
mortie/__init__.py 217 223

Every module is under the ~1,000-line aim. The largest in the package after this is pandas.py at 1,020 (untouched, pre-existing, and the only overage).

One number in #170's rationale has moved since it was written: it cites geometry.py at 971 with "29 lines of headroom" and convert.py at 860. Those were pre-#169-merge figures. On main at 8d4eb0d, geometry.py is 772 and convert.py is 860 (unchanged). The size-pressure argument still holds — geometry.py drops another 170 lines here — the specific headroom figure is just stale. Nothing else in the issue's scope table disagreed with what is on main: all five functions were where it said, including children_of in orders.py rather than the deleted tools.py.

Module docstrings and one comment were updated, and this does not weaken the pure-move claim. The move falsifies four statements that were true before it: coverage.py's and moc.py's "each scalar op sits beside its plural batch twin", orders.py's listing of children_of among its own functions, and a coverage.py comment naming mortie.moc.moc_to_order / mocs_to_orders as one pair. Shipping a phase that leaves those standing would be shipping a known-false doc. The verifier is unaffected by design — top_level_defs skips the module docstring explicitly ("Imports and the module docstring are the two exceptions: the split rewrites both by design"), and a comment between two definitions belongs to no definition's source segment. geometry.py's module docstring never mentioned from_wkbs, so it is untouched.

The same standard caught three more on review, fixed in 2b8dc3e: batch.py's new claim that "each side's docstring carries a See Also to the other" is 3/5 at this commit — morton_coverage_moc and from_wkb have no link back yet — so it now states the pairing as phase 2's obligation instead of as fact; and moc.py's "every bulk operator in the package now sits" there, with coverage.py's "every other bulk operator" and orders.py's "the package's other plural operators", all overreach past mortie.arrow.from_wkbs and mortie.arrow.polygons_to_morton_mocs, which are bulk operators that deliberately stay in the pyarrow skin (issue #154) — each is now qualified to exclude it. All four are module docstrings, so the verifier is unaffected for the same reason and the run below is unchanged.

Tests: five import lines rewired, no test definition moved. test_wkb_batch.py, test_wkb_basins.py and test_arrow_wkb_batch.py import from_wkbs from its module; test_moc_batch.py::test_budget_default_is_the_single_flat_cover_threshold asserts on moc.mocs_to_orders.__defaults__ and now asserts on batch.mocs_to_orders.__defaults__ — the same assertion against the same object. That is the whole test diff. No test module is split here, so the verifier's documented blind spot for test splits (the one that made #159's phase 3 review-gated) does not apply to this PR: there is no unmodelled test move to review-gate.

Phase 2 — what landed (2763877)

The two missing backlinks exist now. morton_coverage_moc (coverage.py) and from_wkb (geometry.py) each gained a See Also entry naming their plural; the other three scalars already had one. All five scalar-side entries are spelled by module path (mortie.batch.polygons_to_morton_mocs, mortie.batch.from_wkbs, mortie.batch.mocs_to_orders, mortie.batch.common_ancestors, mortie.batch.children_of), and all seven cross-module targets in batch.py's own See Also sections — the five scalars plus mortie.orders.clip2order and mortie.moc.split_base_cells — are likewise qualified, retiring the unqualified spellings left over from when each pair shared a module. batch.py's module docstring now states the pairing as fact instead of as phase 2's obligation.

Inline prose references are deliberately left short. The :func: mentions inside docstring prose (:func:moc_to_order in `mocs_to_orders`'s Notes, and the like) keep the bare scalar name: every such docstring's `See Also` carries the qualified path once, and repeating `mortie.moc.` at each of the ~20 prose mentions adds length without information. The one exception is `geometry.py`'s private `_wkb_bytes`, whose docstring names `from_wkbs` in prose with **no** `See Also` to carry the path — that lone pointer is now `:func:`mortie.batch.from_wkbs.

The SPLITS entry retired in this same commit, per the reasoning in "What happens to this split's SPLITS entry at phase 2" below, with a RETIRED (issue #170, …) note in the file mirroring the #159 retirement's, and the header docstring's tenses corrected to match. python benchmarks/verify_pure_move.py --base origin/main at this commit exits 0 checking only the public surface (__all__: 69 names, all resolvable, equal to origin/main's) — the strict-arm run of record remains the one pasted below, at ef78f7e.

Gates at this commit: pytest -q 1334 passed / 16 skipped, flake8 select-gate clean, numpydoc lint mortie/*.py clean, ruff check mortie at the baseline 13, style pass at the baseline 61 lines.

Phase 3 — what landed (8798a8d)

The partition is exactly the table below ("Docs partition"), realized: docs/api/batch.md added with the five members (prose modeled on the other pages: arity consolidation, the arrow skins staying put, names flat on the package), the five stale entries removed from coverage.md / geometry.md / moc.md / orders.md, each of those four pages' prose gaining a one-line pointer at mortie.batch, and moc.md's false "so each scalar op sits beside its plural batch twin" corrected to match what phase 1 already fixed in mortie/moc.py itself. batch slots into mkdocs.yml's nav after geometry — the last of its four source modules in nav order.

Verified against the rendered site, not just the sources. uv sync --group docs && uv run mkdocs build --strict (the Docs job's exact commands) is green locally; the built site has all five id="mortie.batch.<name>" anchors on the batch page, zero stale anchors on the four old pages, and mortie.arrow.from_wkbs still rendered on arrow.md.

The silent-drop failure mode now has a CI gate: mortie/tests/test_docs_api_pages.py, following test_spec_page.py's precedent of pinning a docs page from the suite. It parses every docs/api/*.md (all 12 declare an explicit members: list), asserts each listed name resolves as an attribute of the page's module, and rejects duplicate entries. Mutation-tested: re-adding polygons_to_morton_mocs to coverage.md fails exactly that page's case. This is what turns the review finding "phase 3 has no CI backstop" into a permanent property rather than a one-off hand check. The post-phase-4 review round tightened it in 98ce6c7: the pin is scoped to the stale-entry direction in its own docstring (the reverse — a member deleted from its old page and never added to a new one — is deliberately unpinned; see question 4), a page with two ::: module blocks now fails loud instead of mis-associating its members, and a missing/empty docs/api fails a standalone guard test instead of silently collecting zero cases.

Phase 4 — what landed (7d592fb)

benchmarks/verify_pure_move.py deleted, per espg's ruling (grounds in "Why the file is deleted at phase 4" below, each re-checked at head: the only grep -rn verify_pure_move hit outside the built-site/venv trees was the file's own usage line). The verification of record stands in this body; git log --follow reaches the file by path.

The verifier

benchmarks/verify_pure_move.py gets four changes, and one of them is a retirement that needs saying out loud.

1. #159's SPLITS and SPLIT_BASES entries are retired, because they cannot run on a post-merge main. On origin/main at 8d4eb0d, python benchmarks/verify_pure_move.py --base origin/main exits 1 with 26 failures before this PR touches anything:

origin/main:mortie/tools.py is not reachable in this clone — check --base
011816ca…:mortie/geometry.py: _BACKEND is not in origin/main:mortie/geometry.py — the pin is not a pure import rewire of it
… 24 more of the same shape …
mortie/geometry.py@011816c vs origin/main: MISMATCH — 25 failure(s), listed below

Neither is a merge that went wrong. mortie/tools.py simply does not exist on main any more, so its source side cannot be indexed at all. And 011816ca is still reachable — espg's merge commit did exactly its job — but check_pinned_bases states the pin relative to --base, and after the merge origin/main:mortie/geometry.py is the post-split file that no longer holds the 25 definitions dissolve.py and codec.py took. That is a structural end of life, not a bug: it is the one the SPLIT_BASES comment block already specified ("delete the entry and the split's SPLITS arm once the move has landed and the check has served its purpose"). Worth noting as corroboration that the tool behaved correctly here: it printed MISMATCH — 25 failure(s) rather than a false equality line, which is precisely what 634f114 landed in #169's fold to prevent.

011816ca is not repointed anywhere. It served #169; PR #169's body is where its passing run is recorded. The comment block explaining all of this is kept in the file rather than deleted, and the caveat is generalised so the next split does not have to rediscover it.

#170 needs no pin of its own. Its four sources are read at --base = origin/main = the post-#159 merge 8d4eb0d, which is exactly the commit this branch was cut from and the tree the move was made against. Nothing in this PR edits a source module before moving out of it, so there is no earlier phase for a pin to isolate. orders.py and geometry.py being #169 products does not change that — #169's import rewiring is already in --base, so the strict arm compares this move against a tree that already carries it, and the fourth arm (whose whole job is tolerating such rewiring across a pin) has nothing to tolerate.

That argument was checked rather than left as reasoning, and it holds. With SPLIT_BASES = {} the fourth arm is a genuine no-op, not a hidden pass: check_pinned_bases("origin/main") returns [] and prints nothing. The worry the absent arm would otherwise cover — body-level import rewiring riding along inside a moved definition — is caught by the strict arm anyway: inserting a body-level from .moc import moc_and into the moved common_ancestors gives mortie/batch.py: common_ancestors differs from origin/main:mortie/moc.py (AST), exit 1. And there is none in the diff to catch: the only body-level import changes in the whole PR are arrow.py's two, and arrow.py is neither a source nor a destination, so it was never in this arm's remit (pytest is the gate there). Module-level imports in all four sources are unchanged apart from line numbers, and every one is still used — no new F401.

2. SPLITS keys may now be a tuple of sources. The tool's model was one source → N destinations. This split is four sources → one new destination (plus the four staying put), so batch.py is a destination of four different sources and every arm would otherwise report the other three's functions as "not a move". index_sources indexes a split's sources as one union, and a name bound in two of them is a failure, not an arbitrary pick — the union would otherwise compare one module's definition against the wrong original. (There are no such collisions here; the four sources share no top-level name.) A bare string key still works as shorthand for a one-source split.

3. top_level_defs now indexes attribute docstrings. mortie/coverage.py carries one — the bare string documenting RingValidity — and the scanner reported it as Expr at line 484: is not comparable, which would fail every run touching that module. It is now keyed RingValidity.__doc__, a spelling no Python name can collide with, so it is compared verbatim and has to land wherever its definition lands. Reporting it unhandled would block the gate; skipping it would let a docstring change ride along unseen; keying it does neither.

4. A stay-put destination must keep its own definitions. Found by review, fixed in de5dd11, and it is a hole (2) opened rather than a refinement. Merging the four sources into one old index also merged their identities: origin[name] recorded which source a definition came from, but nothing asserted it, so a definition landing in any destination of the set passed. Demonstrated on this branch — lift polygons_to_morton_mocs out of batch.py, append it verbatim to mortie/orders.py, and honestly rewire __init__.py to from .orders import polygons_to_morton_mocs so the package still imports and __all__ is untouched:

mortie/coverage.py@origin/main, …: 59/59 definitions accounted for across mortie/batch.py, …
__all__: 69 names, all resolvable, equal to origin/main's

Pure move verified.

Exit 0, with a coverage.py function living in orders.py. The pre-#170 one-source model caught exactly this (orders.py: polygons_to_morton_mocs is not a move — no such definition in origin/main:mortie/orders.py); the union could not. The fix asserts the provenance already collected: a destination that is also a source must keep its own definitions, and only a destination outside the source set — batch.py here — may take one from any source. Same mutation after the fix:

1 failure(s):
  - mortie/orders.py: polygons_to_morton_mocs came from origin/main:mortie/coverage.py — a stay-put destination gained another source's definition

Exit 1. The real tree is unaffected: 0 violations, and the run of record below is byte-identical before and after (git write-tree over the mutation cycle returns the same 19cb19a at both ends). The collision half of change (2) was checked the same way and does work as claimed — a base where moc.py and orders.py both bind the same name reports the split's sources are ambiguous, exit 1.

A limitations note was added in ef78f7e for a keying wart in change (3): an attribute docstring is keyed to the last named definition above it, and a skipped import between the two does not break that association, so a stray top-level string following an import is keyed to a name it does not document. It fails loud rather than open — the mis-keyed name is compared like any other, and a second such string comes back unhandled — and mortie has none, so this is documentation, not a fix.

The verification of record

verify_pure_move.py is deleted in phase 4, so this output cannot be re-run against merged main. It is pasted here verbatim as the permanent record of the move's verification. Complete run, python benchmarks/verify_pure_move.py --base origin/main at the fold head ef78f7e, stdout and stderr, nothing elided — identical, line for line, to the same run at the phase-1 commit e2b0281 before the provenance arm in (4) was added:

mortie/coverage.py@origin/main, mortie/geometry.py@origin/main, mortie/moc.py@origin/main, mortie/orders.py@origin/main: 59/59 definitions accounted for across mortie/batch.py, mortie/coverage.py, mortie/geometry.py, mortie/moc.py, mortie/orders.py
__all__: 69 names, all resolvable, equal to origin/main's

Pure move verified.

Exit status 0. That is every line the run produced — the fourth (pinned-base, "equal modulo imports") arm prints nothing here because SPLIT_BASES is empty for this split, which is the same fact as "no pin was needed" above, not a missing check. So the four arms report, in order: 59/59 definitions accounted for across the five destinations (verbatim by AST and by literal source text), completeness both ways (nothing lost, nothing duplicated, no destination gaining a definition that was not there before — EXPECTED_NEW stays empty, so nothing was introduced by the split — and every stay-put destination keeping its own rather than another source's, per (4)), no pinned bases to check, and __all__ equal to origin/main's at 69 names, every one resolvable — that last one compared as two real subprocess imports of two extracted trees, not as two readings of __init__.py.

59 = 12 (coverage.py, counting RingValidity.__doc__) + 19 (geometry.py) + 13 (moc.py) + 15 (orders.py).

The gaps the tool documents about itself, restated so the record is not read as more than it is: a comment sitting between two top-level definitions belongs to no definition's source segment and is not compared (comments inside a body are), an attribute docstring separated from its definition by an import is keyed to the wrong name (loudly — see (4)), and the import rewiring that accompanies a move is not itself checked — pytest is the other half of that gate, and it is green at the same counts as origin/main.

What happens to this split's SPLITS entry at phase 2

It retired, in the phase-2 commit itself (2763877). Phase 2 edits the See Also of five moved docstrings and of their five scalars, so the moved definitions stop being byte-identical to origin/main and check_moves would report five real differences. The alternative offered — scoping the entry to phase 1's head via SPLIT_BASES — does not work here, and it is worth being precise about why rather than leaving it as a judgement call:

  • Pinning the sources at phase 1's head is not merely insufficient for phase 2, it is incoherent: the move has already happened at that commit, so the pinned sources are post-move and hold none of the five. There is no "both sides" to edit — mortie/moc.py stops containing common_ancestors in this very commit. Run against a pin on top of e2b0281, all five come back as mortie/batch.py: <name> is not a move — no such definition in mortie/coverage.py, mortie/geometry.py, mortie/moc.py, mortie/orders.py, plus a 54/55 count line. (An earlier draft of this section gave the weaker and factually wrong reason — that the strict arm would compare against moc.py's copy at the pin and phase 2 edits both sides. The conclusion was right; the reason was not.)
  • Pinning would also plant the exact landmine this PR is cleaning up: a branch sha whose arm is loudly broken the day the PR merges.

This does leave phase 2 machine-unverified, and that is the accepted trade: phase 2 is additive docstring text under the numpydoc hard gate and a strict: true docs build, and the alternative — a docstring-blind comparison — would swap a real guarantee for a cosmetic one, which is exactly the #169 failure mode. Phase 4 deletes the tool regardless.

So the pure-move property is established by phase-1 commit e2b0281 plus the run pasted above, and phase 2's commit removes the entry, leaving SPLITS empty with the retirement notes as the file's documentation. That is deliberate: a check that passes vacuously is worse than no check, and this is the failure mode that was found and closed twice on #169. Phase 4 then deletes the file itself.

One alternative ordering was considered and rejected: cross-link first (in the source modules) and move second, which would preserve the pure-move property across both phases. It would require pinning the sources at the cross-link commit — a branch sha, with the same post-merge death — and it inverts the reviewable order, putting See Also entries pointing at a module that does not exist yet. Move-first with an explicit retirement is the honest shape.

Why the file is deleted at phase 4

espg ruled that #170 cleans it up as part of this work. The grounds, each checked against this tree:

  • Nothing references it. grep -rn verify_pure_move over the repo returns exactly one hit: the file's own name. It is not imported, not invoked by another script, and not wired into any workflow under .github/workflows/pytest does not collect it either (it is benchmarks/verify_pure_move.py, not a test_* module).
  • It is broken on main, not merely idle. Its default --base origin/main run exits 1 with 26 failures, as shown above, because SPLITS names mortie/tools.py and Split tools.py into convert/orders/buffer (issue #159) #169's merge deleted that file. Inert would be tolerable; a checked-in gate that fails by default is a trap for whoever runs it next.
  • It stays recoverable. git log --follow -- benchmarks/verify_pure_move.py reaches it by path, and this PR is where the last working configuration and its output are recorded.
  • Anyone resurrecting it should take the merged version, not rewrite it. It encodes two real holes found during Split tools.py into convert/orders/buffer (issue #159) #169 — the pinned-base arm (check_pinned_bases, which closes the seam where a change made by an earlier split sits in both the pin and the destination and no arm can see it) and the stdout fix that stops a failing pinned-base run from printing an equality line that reads as a pass. This PR adds a third (multi-source splits) and a fourth (attribute docstrings). A from-scratch rewrite would re-earn all four.

Scope: only verify_pure_move.py is deleted. benchmarks/ holds eleven other one-shot scripts, including verify_wkb_corpus_parity.py; none is touched, and none was assessed as part of this issue.

Docs partition (phase 3, landed as 8798a8d)

The moved entries came out of their pages' explicit members: lists and went into docs/api/batch.md, exactly as the #159 review verified its own partition (19 → 7 + 10 + 2):

page before after
docs/api/coverage.md 3 2
docs/api/moc.md 12 10
docs/api/orders.md 10 9
docs/api/geometry.md 7 6
docs/api/batch.md 5
total 32 32 (27 + 5)

docs/api/arrow.md keeps its own polygons_to_morton_mocs and from_wkbs entries — those are the skin's, and they are unaffected.

How it was tested

All gates run against this branch's head (phase 4, 7d592fb), with the origin/main baseline beside each:

gate origin/main this branch
pytest -q 1334 passed, 16 skipped 1347 passed, 16 skipped (+13: the new docs-partition test — one case per docs/api page plus its pages-found guard)
flake8 mortie --select=E9,F63,F7,F82 clean clean
ruff check mortie 13 errors 13 errors (identical set; the new test file is clean)
ruff check mortie --select=E,F,W,I --ignore=E501 (the PR bot's selection) 9 errors 9 errors (identical set)
flake8 mortie --max-line-length=88 (CI's non-blocking style pass) 61 lines 61 lines
numpydoc lint mortie/*.py clean clean
uv run mkdocs build --strict (the Docs job's command) green green, with the five members verified present on the rendered batch page and absent from the four old pages
python benchmarks/verify_pure_move.py --base origin/main exit 1, 26 failures (see above) exit 0 at the phase-1 fold head ef78f7e (strict-arm run of record, pasted above) and at phase 2's 2763877 (public-surface arm only, entry retired); file deleted at phase 4

Pre-existing findings, flagged not fixed (§4): all 13 ruff findings are on origin/main too and none is in a file this PR touches — D205 at mortie/dissolve.py:522, F841 at mortie/convert.py:670 (both tracked on #151), plus D209/D403 in _healpix.py and seven in mortie/tests/. The suite was run with the Rust extension rebuilt from this branch (maturin develop --release); no Rust source is touched by this PR.

Questions for review

  1. Retiring Split tools.py and geometry.py into domain modules mirroring the Rust tree #159's verifier entries inside this PR. It is unavoidable — the tool cannot be used at all otherwise, since origin/main:mortie/tools.py does not resolve — and the script's own comment documents it as the entries' end of life. But it is the deletion of someone else's verification, so it should be a conscious call rather than something noticed in the diff. Flagging it as such. (The file's deletion at phase 4 is not a question — espg ruled it; the justification is in the section above for the record.)
  2. Ordering inside batch.py. The five are laid out coverage → wkb → moc → orders, mirroring the Rust coverage/batch.rs, wkb/batch.rs, moc/batch.rs, decimal_morton/batch.rs grouping within the one file. No section comments were added in phase 1 to keep the move minimal; say the word if the grouping should be marked in the source rather than only in the module docstring.
  3. batch in __all__. Left out, since Resurrect mortie/batch.py as the consolidated home for the bulk operators, with cross-linked scalar/plural docstrings #170 requires the public surface to be unchanged and the verifier asserts it. If mortie.batch should be listed the way mortie.geometry is, that is a deliberate surface change and belongs in its own commit with the check updated to match.
  4. The docs-partition pin is a new permanent test, and it is one-directional. test_docs_api_pages.py reads docs/api/*.md from the repo tree at suite time, exactly as test_spec_page.py already does for the spec page — but it is a gate this issue did not explicitly ask for (it grew out of the phase-1 review's "phase 3 has no CI backstop" finding). It pins the stale-entry direction only; the reverse — requiring every public name to appear on some page — was raised by the post-phase-4 review and deliberately not taken, since it means writing a roster of the 11 currently-undocumented public names into the suite: a docs-policy decision, standing for you. If pinning the partition from the suite is unwanted at all, deleting that one file reverts it cleanly.
  5. Prose :func: references keep the bare scalar name (phase 2, reasoning above): the qualified path lives once per docstring in See Also rather than at every prose mention. Say the word if full mortie.<module>.<name> spellings are wanted inline as well.
  6. See Also renders as plain (unlinked) text on the docs site — a package-wide property, not specific to the ten new cross-links, verified in detail by the post-phase-4 review: griffe's numpy parser has no See Also section type, an autoref inside See Also is refused by the numpydoc lint hard gate, and an autoref in Notes/prose works end to end (real <a class="autorefs">, lint and --strict green). Left as the flat convention here since forking it for just the scalar/plural pairs would be inconsistent; adopting the prose-autoref pattern (package-wide, or for these pairs) is your call and would be a mechanical follow-up.

@espg espg added the implement label Aug 8, 2026
@codecov

codecov Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.58%. Comparing base (8d4eb0d) to head (98ce6c7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #172      +/-   ##
==========================================
+ Coverage   95.56%   95.58%   +0.01%     
==========================================
  Files          16       17       +1     
  Lines        1693     1698       +5     
==========================================
+ Hits         1618     1623       +5     
  Misses         75       75              
Flag Coverage Δ
unittests 95.58% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
mortie/__init__.py 90.32% <100.00%> (+0.32%) ⬆️
mortie/arrow.py 99.36% <100.00%> (ø)
mortie/batch.py 100.00% <100.00%> (ø)
mortie/coverage.py 96.90% <ø> (-0.27%) ⬇️
mortie/geometry.py 100.00% <ø> (ø)
mortie/moc.py 100.00% <ø> (ø)
mortie/orders.py 97.67% <ø> (-0.25%) ⬇️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d4eb0d...98ce6c7. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq

codspeed-hq Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will improve performance by 10.51%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
✅ 69 untouched benchmarks
⏩ 1 skipped benchmark1

Performance Changes

Benchmark BASE HEAD Efficiency
fast_norm2mort_orders[6] 298.6 ns 269.4 ns +10.82%
fast_norm2mort_orders[10] 315.3 ns 286.1 ns +10.19%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/170-batch-module (98ce6c7) with main (8d4eb0d)2

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

  2. No successful run was found on main (93b2deb) during the generation of this report, so 8d4eb0d was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Comment thread benchmarks/verify_pure_move.py Outdated
if name in old:
failures.append(
f"{name}: bound in both {origin[name]} and {base}:{src_path} "
"— the split's sources are ambiguous")

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

Of the three verifier changes this is the one that makes the tool measurably weaker, and it is demonstrable rather than theoretical: the tuple-source union erases source→destination provenance, so a definition landing in the wrong module now passes with exit 0.

index_sources merges the four sources into one old dict. origin[name] keeps the provenance, but it is used for failure messages only — nothing downstream asserts that a definition landing in a destination that is also a source is that same source's. So a definition can migrate between two stay-put modules and the run reports a clean move.

Mutation against this commit (e2b0281), tree restored byte-identical afterwards:

  • lift polygons_to_morton_mocs out of batch.py, append it verbatim to mortie/orders.py
  • rewire __init__.py to from .orders import polygons_to_morton_mocs (so the package still imports and __all__ is untouched)
mortie/coverage.py@origin/main, mortie/geometry.py@origin/main, mortie/moc.py@origin/main, mortie/orders.py@origin/main: 59/59 definitions accounted for across mortie/batch.py, mortie/coverage.py, mortie/geometry.py, mortie/moc.py, mortie/orders.py
__all__: 69 names, all resolvable, equal to origin/main's

Pure move verified.

Exit 0 — with a coverage.py function living in orders.py. The pre-#170 one-source model would have caught exactly this (orders.py: polygons_to_morton_mocs is not a move — no such definition in origin/main:mortie/orders.py); the union cannot, because old no longer knows where a name came from.

The information needed to close it is already collected two lines above. Only a destination that is not one of the sources (batch.py here) may take a definition from any source; a stay-put destination must keep its own:

# in check_moves, right after `landed[name] = dst_path`
if (dst_path in src_paths
        and origin[name] != f"{SPLIT_BASES.get(dst_path, default_base)}:{dst_path}"):
    failures.append(
        f"{dst_path}: {name} came from {origin[name]} — a stay-put destination "
        "gained another source's definition")

I ran that contract over this tree: 0 violations, so it costs nothing to add and it fails the mutation above.

The collision half of this change does work as claimed — I built a base where moc.py and orders.py both bind _collision_probe and got _collision_probe: bound in both …:mortie/moc.py and …:mortie/orders.py — the split's sources are ambiguous, exit 1. It is the provenance half that is unguarded.

Not a blocker for phase 1: the five moves are byte-identical by an independent check (I extracted each function from origin/main with ast.get_source_segment and compared SHA-256 against batch.py's copy — all five match, 5383/8904/6017/5222/7342 bytes). But the run pasted in the PR body is offered as the permanent record once phase 4 deletes the file, so it is worth the tool actually asserting what the record is read as claiming.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude

Fixed in de5dd11 — your contract adopted as posted, with the SPLIT_BASES.get(dst_path, default_base) spelling kept so a pinned stay-put destination is compared against its own pin rather than --base.

landed[name] = dst_path
if (dst_path in src_paths
        and origin[name] != f"{SPLIT_BASES.get(dst_path, default_base)}"
                            f":{dst_path}"):
    failures.append(
        f"{dst_path}: {name} came from {origin[name]} — a stay-put "
        "destination gained another source's definition")

M6b re-run, same tool, before and after. Mutation exactly as you specified: polygons_to_morton_mocs lifted out of batch.py, appended verbatim to mortie/orders.py, __init__.py rewired to from .orders import polygons_to_morton_mocs (package imports, __all__ still 69, mortie.polygons_to_morton_mocs.__module__ == "mortie.orders").

Pre-fix tool, mutated tree:

mortie/coverage.py@origin/main, mortie/geometry.py@origin/main, mortie/moc.py@origin/main, mortie/orders.py@origin/main: 59/59 definitions accounted for across mortie/batch.py, mortie/coverage.py, mortie/geometry.py, mortie/moc.py, mortie/orders.py
__all__: 69 names, all resolvable, equal to origin/main's

Pure move verified.
EXIT=0

Post-fix tool, same mutated tree:

1 failure(s):
  - mortie/orders.py: polygons_to_morton_mocs came from origin/main:mortie/coverage.py — a stay-put destination gained another source's definition
mortie/coverage.py@origin/main, mortie/geometry.py@origin/main, mortie/moc.py@origin/main, mortie/orders.py@origin/main: 59/59 definitions accounted for across mortie/batch.py, mortie/coverage.py, mortie/geometry.py, mortie/moc.py, mortie/orders.py
__all__: 69 names, all resolvable, equal to origin/main's
EXIT=1

Post-fix tool, unmutated tree — unchanged, and byte-for-byte the run pasted in the PR body:

mortie/coverage.py@origin/main, mortie/geometry.py@origin/main, mortie/moc.py@origin/main, mortie/orders.py@origin/main: 59/59 definitions accounted for across mortie/batch.py, mortie/coverage.py, mortie/geometry.py, mortie/moc.py, mortie/orders.py
__all__: 69 names, all resolvable, equal to origin/main's

Pure move verified.
EXIT=0

Tree restored byte-identical, proved rather than asserted: git write-tree over the whole working tree returns 19cb19aa7c3f81472c5b2be2b100ff42b66c4647 both before the mutation cycle and after it, and git status --short afterwards lists only benchmarks/verify_pure_move.py.

Nothing else was weakened. The count line still prints alongside the failure — that is pre-existing and unchanged by this PR, and Pure move verified. correctly does not print. Zero violations against the real tree, so 59/59 and exit 0 both still hold.

Documentation kept level with the arm: the module docstring's claim (2) now says the union keeps each definition's source and that a stay-put destination must keep its own; check_moves carries the rationale (the one-source model got this for free, the union has to say it); and index_sources's Returns no longer describes origin as being for failure messages only.

Gates after the fix: pytest 1334 passed / 16 skipped, flake8 mortie --select=E9,F63,F7,F82 clean, ruff check mortie 13 (identical set, diffed line by line against e2b0281), ruff check benchmarks/verify_pure_move.py clean, numpydoc lint clean on every touched module.

Comment thread mortie/batch.py Outdated
:mod:`mortie.moc`, :mod:`mortie.orders` and :mod:`mortie.geometry`, so "what is
batched?" had four answers and every new twin landed in whichever of those
modules was furthest from the size aim. The scalar/plural pair stays navigable
because each side's docstring carries a ``See Also`` to the other.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

This sentence is false for two of the five pairs at this commit, and phase 1 is the commit that made it false. Phase 1 correctly removed three now-untrue statements (coverage.py's and moc.py's "each scalar op sits beside its plural batch twin", orders.py's listing of children_of) under the PR body's own standard — "shipping a phase that leaves those standing would be shipping a known-false doc" — and then added this one.

Actual See Also state on e2b0281, both sides of each pair:

pair plural side (batch.py) scalar side
polygons_to_morton_mocsmorton_coverage_moc morton_coverage_moc coverage.py:339morton_coverage, compress_mocno link back
from_wkbsfrom_wkb from_wkb geometry.py:401from_geometryno link back
mocs_to_ordersmoc_to_order moc.py:101
common_ancestorscommon_ancestor moc.py:343
children_ofgenerate_morton_children orders.py:404

So it is 3/5, not 5/5 — and the two missing links are precisely the two the PR body says phase 2 will add.

The second-order effect matters more than the sentence: phase 1 turns ten See Also targets into unqualified references to a module they no longer live in — seven in batch.py (morton_coverage_moc, from_wkb, moc_to_order, common_ancestor, split_base_cells, generate_morton_children, clip2order; only polygons_to_morton_mocs still resolves in-module) and three in the sources (moc.moc_to_ordermocs_to_orders, moc.common_ancestorcommon_ancestors, orders.generate_morton_childrenchildren_of). The last three are on pages that render today (docs/api/moc.md, docs/api/orders.md) and point at names their own module no longer has. mkdocs.yml sets strict: true, so I expected the Docs job to fail — it is green, and its log contains zero warnings naming any of the five functions. mkdocstrings degrades these silently, which is the phase-2 retarget's whole point but also means nothing in CI will tell you if phase 2 misses one.

Smallest change that keeps phase 1 honest without pre-empting phase 2: state the intent rather than the fact, e.g. "The scalar/plural pair is kept navigable by a See Also on each side (issue #170)" — or land the sentence with phase 2, where it becomes true.

One more in the same commit, mortie/moc.py lines 17-18: ":mod:mortie.batch (issue #170), where every bulk operator in the package now sits". mortie.arrow.from_wkbs and mortie.arrow.polygons_to_morton_mocs are bulk operators that deliberately do not sit there — this module's own docstring says so three paragraphs down. "every bulk operator in the core surface" would be accurate.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude

Fixed in 2b8dc3e, taking the "state the intent" route rather than landing the sentence with phase 2 — the module docstring should not be silent about the pairing while two of the links are missing. batch.py now reads:

The scalar/plural pair is kept navigable by a See Also on each side, which phase 2 of issue #170 completes: two of the five scalars (mortie.coverage.morton_coverage_moc and mortie.geometry.from_wkb) carry no link back yet, and the targets below are still spelled unqualified, from when each pair shared a module.

No See Also block was touched, so the pure-move property is intact — verify_pure_move.py still reports 59/59, exit 0.

moc.py fixed, and two more of the same claim with it. "every bulk operator in the package now sits" is now "every bulk operator outside the pyarrow skin", naming mortie.arrow.from_wkbs / mortie.arrow.polygons_to_morton_mocs as the exclusion. Phase 1 spelled the same overreach twice more, so they went in the same commit: coverage.py's "consolidated by arity with every other bulk operator" → "every other bulk operator the pyarrow skin does not own", and orders.py's "with the package's other plural operators" → "the pyarrow skin's aside". All four are module docstrings, so the verifier is unaffected for the reason it documents.

The ten unqualified See Also targets: assessed, and they do not render wrong — because they never rendered as references at all. mkdocstrings' numpy handler emits See Also as an admonition of plain text, not as cross-references, so an unqualified target that no longer resolves in-module renders exactly as one that does. From the published 0.9.5 build of docs/api/moc.md (the pre-move tree, where mocs_to_orders was in moc.py):

<details class="see-also" open>
  <summary>See Also</summary>
  <p>morton_coverage : flat single-order cover (post-hoc large-cover warning).
     mocs_to_orders : the ragged batch form (many MOCs in one call).</p>
</details>

No <a href> anywhere in the block — same for every one of the seven See Also sections on that page. So there is no broken link to fix, which is also why strict: true is silent: there is nothing for it to fail on. That makes phase 2's retarget a readability fix (the reader can no longer tell which module the name is in) rather than a rendering one, and it confirms your point that CI will not tell phase 2 if it misses one. Left as-is here, since editing those blocks is phase 2 and would break the pure-move property phase 1 rests on; the module docstring now flags the unqualified spelling explicitly so phase 2 has the list.

docs/api/moc.md's own stale prose line is a separate, docs-side instance of the same sentence — recorded on the phase-3 thread and now in the PR body's phase-3 checklist item, not fixed here.

Comment thread benchmarks/verify_pure_move.py Outdated
# already *in* ``--base``, so the strict arm compares this move against a tree
# that already carries it, and the fourth arm (whose whole job is to tolerate
# such rewiring across a pin) has nothing to tolerate.
SPLIT_BASES = {}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

The empty-SPLIT_BASES argument checks out; the PR body's stated reason for retiring this split's SPLITS entry at phase 2 does not.

Verified, so the record says so plainly:

  • With SPLIT_BASES = {} the fourth arm is a genuine no-op rather than a hidden pass — check_pinned_bases("origin/main") returns [] and prints nothing.
  • I tested the specific worry directly rather than reasoning about it. Inserting a body-level from .moc import moc_and into the moved common_ancestors in batch.py is caught by the strict arm: mortie/batch.py: common_ancestors differs from origin/main:mortie/moc.py (AST), exit 1. So no body-level import rewiring in this move could have hidden behind the absent arm.
  • There is none in the diff anyway. The only body-level import changes in the whole PR are the two in arrow.py (from .coverage import polygons_to_morton_mocs / from .geometry import from_wkbsfrom .batch import …), and arrow.py is neither a source nor a destination, so it was never in this arm's remit — pytest is the gate there, and it is green at 1334 passed / 16 skipped, matching origin/main.
  • Module-level imports in all four sources are unchanged apart from line numbers, and every one is still used (no new F401; ruff check mortie is 13 findings, the identical pre-existing set).

The mis-statement is in the PR body, under "What happens to this split's SPLITS entry at phase 2":

Pinning the sources at phase 1's head does not make phase 2 pass. The strict arm compares batch.py's common_ancestors against moc.py's at the pin, and phase 2 edits both sides.

At phase 1's head mortie/moc.py does not contain common_ancestors — it left in this very commit — so there is no "both sides" to edit. I ran it, pinning the sources at a commit on top of e2b0281: all five come back as mortie/batch.py: <name> is not a move — no such definition in mortie/coverage.py, mortie/geometry.py, mortie/moc.py, mortie/orders.py, plus a 54/55 count line.

The conclusion is right and the true reason is stronger than the one given: a pin at phase 1's head is not merely insufficient for phase 2, it is incoherent for a move that has already happened — the pinned sources are post-move and hold none of the five. Worth correcting in the body, since the body is the permanent record once phase 4 deletes this file.

On whether retiring the entry leaves phase 2 machine-unverified: it does, and I judge that acceptable. Phase 2 is additive docstring text under the numpydoc hard gate and a strict: true docs build; the alternative (a docstring-blind comparison) would trade a real guarantee for a cosmetic one, which is the #169 failure mode. Phase 4 deletes the tool regardless.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude

Both corrections are in the PR body; no code change, so no sha.

The retirement reasoning is replaced with the true one. The bullet now reads that pinning the sources at phase 1's head is not merely insufficient for phase 2 but incoherent — the move has already happened at that commit, so the pinned sources are post-move and hold none of the five, and there is no "both sides" to edit because mortie/moc.py stops containing common_ancestors in this very commit. Your run is quoted for it: all five come back as mortie/batch.py: <name> is not a move — no such definition in mortie/coverage.py, mortie/geometry.py, mortie/moc.py, mortie/orders.py, plus a 54/55 count line. The old bullet is called out as wrong in the same place rather than quietly swapped, since the body is the permanent record once phase 4 deletes the file. The conclusion — the entry retires in the phase-2 commit — is unchanged.

Your judgement on machine-unverification is recorded, not reopened. The body now states plainly that retiring the entry leaves phase 2 machine-unverified and that this is the accepted trade: additive docstring text under the numpydoc hard gate and a strict: true docs build, against a docstring-blind comparison that would swap a real guarantee for a cosmetic one — the #169 failure mode. Phase 4 deletes the tool regardless.

The empty-SPLIT_BASES argument is recorded as verified, in the paragraph that previously only reasoned about it: the fourth arm is a genuine no-op (check_pinned_bases("origin/main") returns [] and prints nothing); your M9 mutation — a body-level from .moc import moc_and inserted into the moved common_ancestors — is caught by the strict arm (mortie/batch.py: common_ancestors differs from origin/main:mortie/moc.py (AST), exit 1), so no body-level import rewiring could have hidden behind the absent arm; and there is none in the diff anyway, arrow.py's two being outside this arm's remit. That retires the worry the "no pin needed" paragraph raised on its own behalf.

One thing your run of it did change: the provenance hole you filed alongside is now closed, so the tool the body describes has four changes rather than three — see the reply on that thread.

Comment thread mortie/__init__.py
# Bulk (plural) twins of the scalar operators, consolidated by arity out of
# coverage / geometry / moc / orders (issue #170). The flat package names below
# are unchanged -- only the submodule they live in moved.
from .batch import (

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

Phase-3 obligation, recorded here because CI will not remind you. Not a phase-1 defect — the PR schedules this correctly, and I recomputed the partition table and it is right (coverage 3, geometry 7, moc 12, orders 10 = 32; docs/api/arrow.md's own two entries are a separate axis and unaffected). But the failure mode of phase 3 is silent, so it needs its own verification.

As of this commit four docs/api pages list members: their module no longer has. Checked by importing each mortie.<page> and hasattr-ing every listed member:

page members: entry no longer on the module
docs/api/coverage.md polygons_to_morton_mocs
docs/api/geometry.md from_wkbs
docs/api/moc.md mocs_to_orders, common_ancestors
docs/api/orders.md children_of

mkdocs.yml sets strict: true, so I expected the Docs job to fail on this. It does not: Build (verify only) on e2b0281 is green, and its log contains zero warnings naming any of the five. mkdocstrings drops an unresolvable members: entry silently. Consequences:

  • Between phase 1 and phase 3 the five functions are simply absent from the rendered sitemortie.polygons_to_morton_mocs and friends have no API page at all right now.
  • If phase 3 misses a page — leaves a stale entry, or forgets one in batch.md — nothing in CI will say so. Worth an explicit check in the phase-3 commit, e.g. asserting every members: name resolves on its page's module (a five-line script over docs/api/*.md, which is what I ran).

Also for the phase-3 sweep: docs/api/moc.md's prose line 5 still reads "Split out of mortie.coverage by domain (issue #156) so each scalar op sits beside its plural batch twin" — the same sentence phase 1 correctly rewrote in mortie/moc.py's module docstring, still standing in the page that renders it.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude

Recorded as a phase-3 obligation, not fixed here — agreed it is not a phase-1 defect. No code change, so no sha.

The PR body's phase-3 checklist item now carries the whole of it explicitly, so whoever does phase 3 cannot read past it:

  • the four pages and their five now-missing members, by name — docs/api/coverage.md (polygons_to_morton_mocs), docs/api/geometry.md (from_wkbs), docs/api/moc.md (mocs_to_orders, common_ancestors), docs/api/orders.md (children_of);
  • that strict: true does not catch this — mkdocstrings drops an unresolvable members: entry silently, the Docs job is green with zero warnings naming any of the five, so phase 3 has no CI backstop and a missed page will not be reported;
  • the consequence in the meantime: those five functions have no rendered API page at all between phase 1 and phase 3;
  • the instruction that the phase-3 commit assert every members: name in docs/api/*.md resolves on its page's module — your five-line check, made part of the phase rather than a one-off;
  • and docs/api/moc.md's prose line 5, still carrying the "so each scalar op sits beside its plural batch twin" sentence that phase 1 corrected in mortie/moc.py itself.

Your recomputation of the partition table (coverage 3, geometry 7, moc 12, orders 10 = 32, docs/api/arrow.md a separate axis) matches the table in the body; it is left as-is.

Comment thread mortie/tests/test_docs_api_pages.py Outdated
no longer had, and nothing in CI said so. This test is the backstop: every
``members:`` name in ``docs/api/*.md`` must resolve as an attribute of the
module its page documents, so a stale entry (or a move that forgets its docs
page) fails here instead of vanishing from the site.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

The pin is one-directional, and the docstring's parenthetical claims the direction it does not check. "a stale entry (or a move that forgets its docs page) fails here instead of vanishing from the site" — a stale entry does fail here; a member that is never listed vanishes exactly as silently as before, and this test is green while it does.

Mutation at head (7d592fb), tree restored byte-identical afterwards — drop one line, - children_of, from docs/api/batch.md:

$ python -m pytest mortie/tests/test_docs_api_pages.py -q
............                                                             [100%]
12 passed in 0.29s

$ uv run --group docs mkdocs build --strict -d /tmp/site_m3
INFO    -  Documentation built in 1.80 seconds

$ grep -c "mortie.batch.children_of" /tmp/site_m3/api/batch/index.html
0

Suite green, docs build green under strict: true, and children_of has no rendered API page anywhere on the site. That is the same class of silent loss the module docstring opens with, and it is the one a future move is most likely to commit: phase 3 removed five entries from four pages and added five to a fifth, and only the removals are pinned. Had phase 3 shipped the removals and forgotten docs/api/batch.md entirely, the only thing that would have complained is mkdocs --strict about the dangling nav: entry — nothing about the five functions.

Not hypothetical, either. Eleven names in mortie.__all__ are on no docs/api page today:

norm2uniq, uniq2geo, unique2parent, geo2uniq, RingValidity, ring_is_simple,
ring_validity, rank_to_xy, xy_to_rank, MortonIndexType, MortonIndexExtArray

so the reverse direction is already drifting and nothing reports it.

Two ways out, either is fine:

  1. Narrow the claim — drop the parenthetical, and say plainly in the docstring that this pins listed → resolves and not public → listed, so the next reader does not assume a guarantee that is not there.
  2. Close it — add a second assertion that every name in mortie.__all__ which is not a submodule appears on exactly one docs/api page, with the eleven above as a named, comment-justified allowlist. That makes the drift visible and makes adding to the allowlist a deliberate act.

(1) is honest and costs a line; (2) is what would actually have caught phase 3 going wrong in the other direction.


Two smaller things in the same file, neither live today:

  • page_members splits on the first "members:" in the whole file and then re.findalls indented - bullets to EOF, while the module comes from the first ^::: match. A page with prose containing members:, a bulleted list after the mkdocstrings block, or a second ::: block would silently mis-associate rather than fail. Anchoring the scan to the text between the matched ::: line and the next :::/EOF would remove the coupling; all 12 pages happen to be single-block today, which is the only reason it holds.
  • API_DIR = Path(__file__).resolve().parents[2] / "docs" / "api" degrades to a silent skip, not an error, if docs/api is not there: an empty parametrize list makes pytest report 1 skipped, not a failure. test_spec_page.py, the cited precedent, fails loudly instead (read_text raises). A module-level assert API_DIR.is_dir() would make the pin fail rather than evaporate. CI never hits this today — build-wheels.yml's installed-wheel run names three test files explicitly and this is not one of them — so it is a latent trap in a test whose value is being permanent.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude

Fixed in 98ce6c7, taking the narrow-the-docstring route plus both secondary traps:

  • The overclaim is retracted. The module docstring now states the pin covers the stale-entry direction only, and says out loud what is deliberately not checked: "nothing here requires every public name to appear on some page, so a move that deletes a member from its old page and forgets to add it to the new one still vanishes silently — as do the public names that have no docs/api entry today." Your mutation (dropping - children_of from batch.md, suite green) is exactly the case that sentence now owns.
  • The two-block mis-association fails loud now. page_members asserts exactly one ::: module block per page — the repo convention the members:-split parsing assumes — instead of silently crediting a second block's entries to the first module.
  • The empty-glob silent skip is closed. A standalone test_api_pages_found fails when API_DIR is missing or holds no pages, so the parametrized cases can no longer degrade to zero collected tests; the suite is 13 cases now, and it matches test_spec_page.py's fail-loud posture.

The reverse assertion with an allowlist was not taken, and that half stands for espg rather than being my call: pinning it means writing a roster of the 11 currently-undocumented public names into the suite, which is a docs-policy decision (which names should have pages) rather than a drift check — and this PR's scope is the five moved members' partition. It is recorded under "Questions for review" in the PR body so it is a conscious decision, not a diff artifact.

Comment thread mortie/coverage.py
--------
morton_coverage : flat single-order cover.
compress_moc : merge 4-sibling groups in an existing morton set.
mortie.batch.polygons_to_morton_mocs : the batch form (many polygons in

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

Phase 2's qualified paths are correct in the source but are not links on the rendered site, and the obvious way to make them links breaks the numpydoc gate. Worth deciding here rather than discovering it after merge, because navigability is the mitigation the whole arity split rests on (issue #170: "the scalar/bulk split is navigable by cross-linking").

griffe's numpy parser has no See Also section type — the block falls through to an untyped admonition and the entries are emitted as one raw <p>. From the built site at head (site/api/coverage/index.html, this exact docstring):

>See Also</summary>
  <p>morton_coverage : flat single-order cover.
compress_moc : merge 4-sibling groups in an existing morton set.
mortie.batch.polygons_to_morton_mocs : the batch form (many polygons in
    one call).</p>

No <a href>, and the three entries collapse into one run-on paragraph. Same on moc.md, orders.md, geometry.md and all five batch.md entries. So a reader on the coverage page is told the name mortie.batch.polygons_to_morton_mocs and has to go find it; the page-level [mortie.batch](batch.md) pointer phase 3 added is doing the actual navigating.

This rendering is pre-existingmoc_or : union of two covers. was already plain text — so it is not a regression. It matters now because phase 2 is where the cross-link stopped being a convenience and became the deliverable.

The obvious fix does not work, and I checked rather than assuming. mkdocstrings' autoref syntax does resolve these targets (so the anchors are right), but numpydoc's See Also parser rejects the bracket spelling outright:

$ numpydoc lint mortie/coverage.py     # entry rewritten as [mortie.batch.polygons_to_morton_mocs][]
ValueError: Error parsing See Also entry
  '[mortie.batch.polygons_to_morton_mocs][] : the batch form (many polygons'
exit=1

numpydoc lint mortie/*.py is a hard gate in lint.yml, so that spelling is off the table inside See Also.

What does work — verified end to end, both gates green — is one autoref in prose. Adding The batch form is [mortie.batch.children_of][]. to generate_morton_children's existing Notes gives:

$ numpydoc lint mortie/orders.py
exit=0
$ uv run --group docs mkdocs build --strict
INFO    -  Documentation built in 1.80 seconds
<summary>Notes</summary>
  <p>The batch form is <a class="autorefs autorefs-internal"
     title="children_of(words, order, max_cells=None)"
     href="../batch/#mortie.batch.children_of">mortie.batch.children_of</a>.</p>

A real internal link, under strict: true, with the numpydoc gate clean.

Two options, both defensible:

  1. Keep See Also as-is and say so. Add a line to the PR body that the mitigation is source-level naming — the reader of the docstring gets the module path — and that the rendered site links at page granularity only. That closes the gap between what issue Resurrect mortie/batch.py as the consolidated home for the bulk operators, with cross-linked scalar/plural docstrings #170 promised and what ships, without touching code.
  2. Add the prose autoref on the five scalars (and optionally the five plurals), keeping the See Also entry as the numpydoc-correct summary. Ten short lines, all inside sections the numpydoc gate already accepts, and the scalar/plural pair becomes one click on the published docs.

Note this also cuts against phase 2's stated rationale for keeping inline :func: prose mentions bare: those :func: roles do not render either (the batch page shows :mod:mortie.orders and :func:mortie.arrow.from_wkbs literally, role markup and all — pre-existing and out of scope here, but it means prose is currently the unrendered half, not the redundant one).

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude

Left standing for espg — no code change, and here is the case for why, made explicitly rather than by omission:

  • The finding is real and your verification is valuable: See Also renders as one raw paragraph (griffe's numpy parser has no section type for it), autorefs inside See Also are refused by the numpydoc lint hard gate, and autorefs in Notes/prose work end to end. Nothing below disputes any of that.
  • But the flat rendering is a package-wide property, not a phase-2 defect. Every See Also in mortie renders this way — mortie.arrow.from_wkbs's pointer at its core twin included — so the ten cross-links this PR adds are exactly as navigable as every existing one. Moving only the scalar/plural pairs into Notes-with-autorefs would fork the docstring convention: ten functions with a clickable cross-ref in prose plus a duplicate plain entry in See Also (numpydoc's SS/section conventions still want the entry there), while the rest of the package keeps the flat form.
  • The qualified module path is the mitigation that is in scope. Issue Resurrect mortie/batch.py as the consolidated home for the bulk operators, with cross-linked scalar/plural docstrings #170's requirement is that each side's docstring points at the other; mortie.batch.mocs_to_orders as rendered text tells a reader exactly which page to open, which the pre-phase-2 bare mocs_to_orders did not.

The end-to-end-verified alternative (autoref in prose passes numpydoc lint, mkdocs --strict, and produces a real <a class="autorefs">) is recorded in the PR body's "Questions for review" with a link to this thread, so espg can rule on it as a package-wide docs decision — either adopting the prose-autoref pattern everywhere or accepting the flat See Also rendering. If he wants it in this PR, it is a mechanical follow-up commit.

@espg
espg marked this pull request as ready for review August 9, 2026 05:39
@espg espg added the waiting label Aug 9, 2026
@espg
espg merged commit dfcb88f into main Aug 9, 2026
24 checks passed
@espg
espg deleted the claude/170-batch-module branch August 9, 2026 06:04
espg added a commit that referenced this pull request Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resurrect mortie/batch.py as the consolidated home for the bulk operators, with cross-linked scalar/plural docstrings

1 participant