convert tools / coverage / prefix_trie / linestring docstrings to numpydoc

[espg]

Closes #135. Refs #68.

Phase 2 of the repo-wide numpydoc sweep: the four modules deferred from phase 1 (PR #141) to avoid colliding with PR #130 and PR #139. Both of those have merged, so the contention is gone. CLAUDE.md §7 now mandates numpydoc, so this brings the last of mortie/ in line with it and with mkdocstrings' docstring_style: numpy.

Every public function, class and method in the four modules now carries a one-line imperative summary, then Parameters / Returns / Raises / Warns where applicable, with the existing design rationale preserved verbatim in the Extended Summary rather than compressed into parameter tables.

Phases

• mortie/tools.py — convert tools docstrings to numpydoc (issue #135)
• mortie/coverage.py — convert coverage docstrings to numpydoc (issue #135)
• mortie/prefix_trie.py — convert prefix_trie docstrings to numpydoc (issue #135)
• mortie/linestring.py — convert linestring docstrings to numpydoc (issue #135)
• Missing-pandas ImportError wording correction (own commit, see below)
• Adversarial self-review, findings folded in (address self-review findings (issue #135))

Per-module before/after

Two metrics, read together — neither alone is the finish line. ruff never requires a Parameters section to exist, so a file can be ruff-green with no numpydoc structure at all (that was linestring.py's starting state). Conversely tools.py and coverage.py were largely sectioned but carried the highest D counts: formatting defects inside already-structured docstrings.

module	defs+classes	Parameters blocks before → after	ruff D before → after
mortie/tools.py	24	23 → 24	7 → 0
mortie/coverage.py	16	11 → 15	18 → 0
mortie/prefix_trie.py	15	7 → 11	1 → 0
mortie/linestring.py	3	1 → 3	0 → 0

The counts that stop short of the def count are correct, not gaps:

• coverage.py 15/16 — _whole_sphere() takes no arguments, so it has Returns and no Parameters.
• prefix_trie.py 11/15 — the four without a Parameters block are MortonChild.__init__ (constructor arguments are documented on the class, per numpydoc), the mantissa_array and cell_area properties, and __repr__. None takes a documentable argument; all now have summaries, and Returns where they return something.

What changed beyond adding sections

• coverage.py — narrative parsed as parameters. morton_coverage_moc had its multipart/holes paragraph sitting after the Returns header, which numpydoc reads as further return entries. Moved into the Extended Summary, above Parameters. This was a live rendering bug, not a style nit: the self-review confirmed griffe's numpy parser emits four returns entries for that function on main — one real, three bogus, one per line of the stranded paragraph. On the branch it emits one returns plus one raises. The adaptive-stop-criteria lead-in also dangled on a colon directly before the Parameters header; it now names tolerance and max_cells and ends in a period. No words dropped.
• coverage.py — five docstrings converted to r""" (moc_or, moc_and, moc_minus, moc_xor, moc_not) so the a \ b set-difference notation stops needing \\ (ruff D301). The runtime docstring text is byte-identical to main for four of them, and differs only in the summary rewrite for moc_not — the escape change is purely how the literal is spelled, not what it says.
• coverage.py — two two-line summaries collapsed (moc_not, split_base_cells, ruff D205); the displaced clause moved to the first line of the Extended Summary, intact.
• tools.py — geo2mort's summary was Calculates ..., with no period and with the closing quotes on the content line (D400/D401/D209); it is now Compute morton indices from geographic coordinates. Three more summaries gained their missing period (mort2norm, mort2geo, mort2bbox).
• tools.py — section order fixed in res2display and mort2healpix, which had Examples before See Also / Notes. numpydoc renders Examples last.
• linestring.py — the multi-linestring example did not run. >>> [arr.shape for arr in per_line] had no expected output, so it failed under doctest. It now carries the real output, [(10,), (27,)], and both examples moved out of :: literal blocks into plain doctest blocks matching morton_coverage's style.
• Cross-references kept as cross-references. coverage._single_coverage's normalize defers to morton_coverage by name for the full ring-winding contract rather than restating an abridged version — the failure mode PR Convert morton_index, geometry and arrow docstrings to numpydoc #141's review caught on from_wkb/from_wkt, where an apparently-complete table quietly dropped a caveat.

Substantively wrong prose found

Flagged separately from the formatting work, because this is a docstring that says something false rather than one that is merely unstructured.

morton_polygon's Returns on main reads Refined prefix-cells (len <= *n_cells*)., and its n_cells parameter reads Maximum number of cells in the returned list. Neither holds. morton_polygon seeds the frontier with the roots and only ever grows it:

current = list(roots)
count = len(current)
...
while count < n_cells and heap:

so when the trie has more root-level children than the budget, the budget is simply ignored. Reproduced on a 7-root trie: morton_polygon_from_array(m, n_cells=n) returns 7 cells for every n in 1..7. Roots are expanded, never merged, so len(roots) is a floor on the result.

I hit this because I had propagated the same sentence onto geo_morton_polygon and morton_polygon_from_array while filling in their missing return descriptions; the self-review caught it there. All three now describe the root-count floor, and the n_cells lines no longer claim a maximum.

The behavioural question is open and left for review (see "Questions for review" (1)): whether the floor is intended, or morton_polygon should reject n_cells < len(roots) rather than silently overshoot. No code changed either way.

Missing-pandas ImportError wording

Separate commit, correct the missing-pandas ImportError wording (issue #135), so it can be reviewed or reverted independently of the conversion.

The message introduced in PR #141 was factually wrong, not merely unclear. Its final clause:

... or declare it as a mortie extra with `pip install mortie[pandas]`, which
pins pandas as a mortie dependency so it is loaded whenever mortie is imported.

made two false claims. (1) "pins" — the extra is pandas>=2.0 in pyproject.toml, a lower bound, not a pin. (2) "so it is loaded whenever mortie is imported" — equally true after a plain pip install pandas; the eager probe at the bottom of morton_index.py keys off whether pandas is importable, not how it was installed. The clause therefore invented a difference between the two remedies it offered, and could read as implying the bare install leaves the ExtensionArray unavailable. Replaced with:

the morton_index ExtensionArray requires pandas, which is not installed.
Install it with `pip install pandas`, or with `pip install mortie[pandas]` to
declare it as a mortie extra so it is installed alongside mortie in future
environments.

which states the real difference — the extra records the dependency in mortie's metadata, so a fresh install, a lockfile, or another machine picks pandas up automatically.

Updated in all three places that carry it, and re-grepped afterwards to confirm nothing else quotes it (the only remaining hits are in the gitignored htmlcov/ coverage artifacts):

• mortie/morton_index.py — the single string literal. That property is preserved: mortie/pandas.py still reaches it through _require_pandas() rather than duplicating it.
• mortie/tests/test_pandas_module.py — the three assertions matching "pins pandas as a mortie dependency" re-pointed at "installed alongside mortie". Assertions kept, not deleted, and still substring matches rather than whole-paragraph matches, which would be brittle.
• docs/morton_index_datatype.md — the verbatim quote updated to match exactly (verified character for character, 244 chars).

test_message_is_identical_on_every_path already asserts that all three raise paths (mortie.MortonIndexArray, from mortie.morton_index import ..., from mortie.pandas import ...) produce the same string (len(set(msgs)) == 1); it passes.

How it was tested

The diff on the four converted modules is docstring-only, proved two ways per file (the AST check alone is blind to comments — PR #141 found six comment blocks that had migrated into docstrings):

1. Parse both the origin/main version and the branch version, delete every docstring Expr node, ast.unparse both, compare.
2. Extract the tokenize.COMMENT token stream (text only, position ignored) from both and compare.

tools.py:       ast_code_identical=True comments_identical=True (n_comments 123 -> 123)
coverage.py:    ast_code_identical=True comments_identical=True (n_comments  17 ->  17)
prefix_trie.py: ast_code_identical=True comments_identical=True (n_comments  20 ->  20)
linestring.py:  ast_code_identical=True comments_identical=True (n_comments   0 ->   0)

The only non-docstring lines in the four-file diff are eight D202 blank-line deletions in coverage.py (a blank line between a docstring and the first statement).

The ImportError commit is deliberately not docstring-only — it changes a string literal, its assertions and its doc quote — which is why it is a separate commit.

Other gates, all run in a clean venv on this branch, and all re-run after the self-review fixes:

• pytest — 804 passed, 12 skipped, exactly the baseline measured on origin/main @ 3f306a8 before any edit (804 passed, 12 skipped). No movement, so no logic was touched.
• ruff check --select D on each of the four files — clean, exit 0.
• flake8 mortie --select=E9,F63,F7,F82 — clean, exit 0.
• mkdocs build --strict — exit 0, zero warnings. Exit code read directly from the build, not through a pipe.
• doctest.testmod on all four modules — 37 examples, 0 failures. On main linestring.py had 1 failure; it is fixed here.

An independent fresh-context adversarial review re-ran every one of those and reproduced the same numbers, and found nothing on lost rationale (a token-level audit found four main words absent, all deliberate summary rewrites; every issue ref and § link survives), lost or moved comments, the docstring-only claim, non-running examples, r""" text neutrality, or section ordering. Its five diff-scoped findings are folded into 389ce57 and itemised in the review-response comment.

Questions for review

(1) Is morton_polygon's root-count floor intended? Per the "Substantively wrong prose" section above, n_cells is not honoured when the trie has more roots than the budget — you get len(roots) cells back with no error. The docstrings now describe that faithfully, but the two plausible intents point different ways: (a) the floor is inherent (you cannot describe the data in fewer cells than it has root groups) and the docs are now correct as written; (b) silently returning more cells than asked for is a bug and morton_polygon should raise when n_cells < len(roots). I did not change behaviour. If (b), it wants its own issue rather than a docs-only PR.

(2) _normalize_antimeridian_polygon has an unused variable. ruff check mortie/tools.py reports F841 for on_antimeridian at line 975 — assigned from np.sum(...) and never read. It is pre-existing on main (line 945 there) and fixing it is a logic change, so it is deliberately untouched here to keep the diff docstring-only. The neighbouring mort2bbox computes the same quantity as a mask and uses it; this one computes a count and drops it, which looks like the vestige of an earlier version of the check. Worth its own issue?

(3) tools.py is 1354 lines, over CLAUDE.md §4's ~1000-line guidance. It was already 1315 on main; this PR adds 39 lines of docstring and no code. Flagging rather than splitting, since §4 asks for discussion first and a split here would be churn on top of a docs-only change.

(4) MortonChild gained an Attributes section listing the frozen 1.x read surface (characteristic, len, children, nchildren). The prose above it already enumerated those names; the section makes mkdocstrings render them as a table. Say the word if you would rather the prose stayed the single source and the section came out.

(5) mort2healpix's example now calls mortie.mort2healpix(m) rather than the bare mort2healpix(m). The bare form only worked because doctest.testmod runs in the defining module's globals; a reader copying the rendered example after import mortie would have hit a NameError. The example still runs and still prints HEALPix cell 37010 at order 6.

(6) Doctest examples are not executed by CI. The [(10,), (27,)] output now pinned in linestring_coverage is a behavioural assertion nothing runs — there is no --doctest-modules in addopts and no workflow invokes doctest. I verified all 37 examples by hand for this PR, but the next docstring change will not be checked. Adding --doctest-modules is a pyproject.toml change, which this PR is scoped out of; worth a separate issue?

(7) Enforcement is still open from #135 itself — whether to add a numpydoc validation pass to lint.yml, or leave the ruff D rules as the mechanical floor with numpydoc structure as convention. All of mortie/ is converted now, so it is decidable; it is out of scope for this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.90%. Comparing base (3f306a8) to head (389ce57).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #143   +/-   ##
=======================================
  Coverage   94.90%   94.90%           
=======================================
  Files          10       10           
  Lines        1434     1434           
=======================================
  Hits         1361     1361           
  Misses         73       73           

Flag	Coverage Δ
unittests	94.90% <ø> (ø)

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

Files with missing lines	Coverage Δ
mortie/coverage.py	97.67% <ø> (ø)
mortie/linestring.py	96.15% <ø> (ø)
mortie/morton_index.py	93.44% <ø> (ø)
mortie/prefix_trie.py	83.87% <ø> (ø)
mortie/tools.py	98.05% <ø> (ø)

---

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 3f306a8...389ce57. 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]

Merging this PR will not alter performance

✅ 67 untouched benchmarks
⏩ 1 skipped benchmark¹

---

_{Comparing claude/135-numpydoc-phase2 (389ce57) with main (3f306a8)}

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. ↩

[espg]

🤖 from Claude (review)

Fresh-context adversarial self-review of claude/135-numpydoc-phase2 @ 320bd7b. Every claim in the PR body reproduced independently; all four gates re-run. No blocking findings. Seven inline comments, all accuracy or follow-up, none of which need to land before the next phase.

Verified clean

Docstring-only diff. Reproduced both proofs per file, independently of the PR's script — parse origin/main and branch, delete every docstring Expr, ast.unparse, compare; then compare the tokenize.COMMENT token text streams:

tools.py:       code_identical=True  comments_identical=True (123 -> 123)
coverage.py:    code_identical=True  comments_identical=True ( 17 ->  17)
prefix_trie.py: code_identical=True  comments_identical=True ( 20 ->  20)
linestring.py:  code_identical=True  comments_identical=True (  0 ->   0)

No comment text migrated into a docstring; no docstring text migrated into a comment.

No rationale lost. Token-level audit per docstring (every identifier, #NNN issue ref, § cross-link, digit and backslash in the main docstring, checked for presence on the branch). The complete set of main tokens absent on the branch across all four files is four words, all of them deliberate summary rewrites:

function	dropped	why
tools.geo2mort	Calculates	→ Compute (D401)
coverage.moc_not	of	"Complement of a morton cover" → "Complement a morton cover"
coverage.split_base_cells	each, s	"keyed by each group**'s**" → "Each group is keyed by its own"
prefix_trie.MortonChild._compact	branching	→ "Branch on divergence"

Every issue reference survives (#11, #22, #34, #58, #60, #68, #80, #116, #119, #136), as do the spec § cross-links. Read the four flagged sites word by word — _whole_sphere, moc_not, split_base_cells, morton_coverage_moc — and every displaced clause is intact in the Extended Summary.

The morton_coverage_moc relocation was a real bug, not cosmetics. Parsing both versions with griffe's numpy parser (the one mkdocstrings uses) shows what the misplaced paragraph actually did:

MAIN   returns: 'numpy.ndarray'                                              -> 'Sorted 1-D array of mixed-order...'
       returns: 'For **multipart / holes** (lists of rings), all rings...'  -> ''
       returns: 'even-odd descent — disjoint parts union with no...'        -> ''
       returns: 'rings carve holes (a donut is ``[outer, hole]``).'         -> ''
BRANCH returns: 'numpy.ndarray'                                              -> 'Sorted 1-D array of mixed-order...'
       raises:  ValueError

Each line of prose became its own bogus return entry, typed as itself. Fixed here.

The r""" conversions are text-neutral. Compared ast.get_docstring output between main and the branch: moc_or, moc_and, moc_minus, moc_xor are byte-identical. moc_not differs only in the summary/extended-summary rewrite. Readers still see a \ b, confirmed in the rendered site/api/coverage/index.html.

numpydoc structure. Scripted scan of all 40+ docstrings in the four files: zero cases of narrative appearing after a Parameters/Returns/Raises/Warns/See Also header, zero section-ordering violations, Examples last everywhere. Parameter names match signatures throughout. All four modules are rendered by mkdocstrings (docs/api/{tools,coverage,prefix_trie,linestring}.md), so the strict build is a meaningful gate rather than a no-op.

Examples run, and resolve for a reader. mortie.mort2healpix(m) resolves after the >>> import mortie shown in the example — confirmed. The bare mort2healpix(m) form on main only passed because doctest.testmod injects the defining module's globals, so the fix is real.

Gates, re-measured on this branch

gate	result
pytest -q	804 passed, 12 skipped, 10.69s — matches the claim and the main baseline
flake8 mortie --select=E9,F63,F7,F82	exit 0
ruff check --select D (four files)	All checks passed!, exit 0
mkdocs build --strict	exit 0 (read directly: > /dev/null 2>&1; echo $?), no warnings beyond the Material 2.0 banner
doctest.testmod	tools 11/0, coverage 18/0, prefix_trie 0/0, linestring 8/0 — 37 examples, 0 failures
same, on main's versions	linestring 1 failure, fixed here
ruff check (full, four files)	exit 1 — the single pre-existing F841

The F841 in "Questions for review" (1) checks out as pre-existing: on_antimeridian is assigned and never read at mortie/tools.py line 945 on main (line 975 here), inside _normalize_antimeridian_polygon. Untouched here is the right call for a docstring-only diff; agreed it wants its own issue.

Findings

Medium — 1

• prefix_trie.geo_morton_polygon / morton_polygon_from_array: the new Returns text "Refined prefix-cells (len <= n_cells)" states a guarantee the code does not make. morton_polygon_from_array(m, n_cells=1) returns 6 nodes when the trie has 6 roots. Pre-existing wording on morton_polygon, newly copied onto two more public functions. Inline, with repro.

Low — 4 (all inline)

• morton_coverage_moc's new Raises omits the multipart ring-count ValueError from _prep_rings, now that Parameters advertises the multipart form.
• mort2healpix states the same-order precondition in Notes but gained no Raises, while mort2norm and generate_morton_children did.
• _expansion_efficiency: "undefined for a leaf" is actually a negative float, and the single-child case it warns about is unreachable.
• MortonChild's new Raises: the leading-character clause only fires when start_col > 0.

Suggestions — 2 (inline, explicitly out of scope for this PR)

• The newly pinned [(10,), (27,)] in linestring_coverage is a behavioural assertion no CI job runs.
• Nothing enforces that docs/morton_index_datatype.md keeps quoting the real ImportError — the drift this commit exists to fix.

Observations, no action asked

• Eight blank lines were deleted after docstrings in coverage.py (compress_moc, moc_to_order, the four set ops, common_ancestor, split_base_cells) — ruff D202. These are the only lines in the four-file diff that are not docstring content. Whitespace-only, invisible to ast.unparse, so the docstring-only claim stands; noting it only because "docstring-only" is stated as an absolute.
• :func: / :attr: roles render literally on the site. mkdocstrings in Markdown mode does not process Sphinx roles, so :func:moc_or`` publishes as the literal text :func: moc_or — visible in the built `site/`. Thoroughly pre-existing (the `coverage` module docstring on `main` does it, unchanged here) and pervasive, and this PR adding more is consistent with the surrounding code per the repo conventions. Flagging so it is on the record as a repo-wide docs decision, not as something to change in this PR.
• On "Questions for review" (3): the MortonChild Attributes section reads well and renders as a table, but it lists four of the six names the prose above calls the frozen read surface — mantissa_array and cell_area are omitted because they are properties with their own entries. That is defensible numpydoc, just worth being a deliberate choice rather than an accident.
• On "Questions for review" (2): tools.py measures 1348 lines here. Flagging rather than splitting is the right read of the conventions on a docs-only change.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

len <= *n_cells* is not a guarantee the code makes. morton_polygon starts from current = list(roots) and only ever grows that list, so the budget is honoured only once it is already at least the root fan-out. Reproduced on this branch:

>>> lats = np.array([-80., -70., 10., 20., 60., 65., 30., -30.])
>>> lons = np.array([0., 45., 90., 135., 180., -135., -90., -45.])
>>> m = mortie.geo2mort(lats, lons, order=6)
>>> len(split_children(m, max_depth=4))
6
>>> [len(morton_polygon_from_array(m, n_cells=n)) for n in (1, 2, 3, 4)]
[6, 6, 6, 6]

The wording is copied from morton_polygon's own Returns, which is unchanged from main, so the inaccuracy is pre-existing — but this PR newly propagates it onto two more public functions (geo_morton_polygon here, morton_polygon_from_array at line 397), and the n_cells parameter line right above (line 363, "Maximum number of cells in the returned list") overstates it the same way.

Suggested wording for the three Returns blocks:

Refined prefix-cells — at most n_cells, but never fewer than the number of root-level groups, which sets the floor when the fan-out already exceeds the budget.

plus the matching caveat on the n_cells parameter. Since the two new copies are yours, fixing all three together keeps them from drifting apart.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Second copy of the len <= *n_cells* overstatement — see the comment on geo_morton_polygon (line 373) for the repro. Same fix applies here and to n_cells at line 389.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

This new Raises under-claims the multipart path. The Parameters block directly above now advertises "or a list of such arrays for the multipart form", and that path routes through _prep_rings, which raises a fourth ValueError this list does not cover:

if len(lats) != len(lons):
    raise ValueError("lats and lons must have the same number of rings")

"the ring's lats and lons have different lengths" is the within-ring mismatch (_prep_rings, line 104). The ring-count mismatch (line 98) is a distinct failure, and you already worded it correctly on _prep_rings itself (line 93: "If the ring counts differ"). Suggest folding the same clause in here so the two agree.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

mort2healpix has no Raises, but it does raise. The Notes you just relocated here state the precondition — "All input morton indices must be at the same order" — and it is enforced, one frame down: mort2healpix calls mort2norm, which raises

ValueError: Mixed orders in morton array: [...]; use orders_of for per-element orders

This PR added a Raises section to mort2norm (line 606) and to generate_morton_children (line 1154) for exactly this class of contract, so mort2healpix reads as the one that got skipped — and it is the function whose docstring states the constraint without saying what happens when it is violated. Suggest:

    Raises
    ------
    ValueError
        If the words are at mixed orders (propagated from :func:`mort2norm`).

placed before Notes.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Two small inaccuracies in this new parameter note.

(1) "undefined for a leaf" — for a leaf nchildren == 0, so cost == -1 and benefit == node.cell_area; benefit / cost returns a negative float. Nothing is undefined and nothing raises, it just silently returns a meaningless negative score. "negative and meaningless for a leaf" would be exact.

(2) "cost is zero for a single child" describes a state the trie cannot reach. _compact only creates children in the divergence branch (line 178), which is guarded by len(unique) == 1 being false — so nchildren is always 0 or >= 2, never 1. As written the note advertises a ZeroDivisionError no caller can trigger.

Neither is load-bearing; flagging because the sentence is new text and a reader will take it at face value.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

The second clause of this new Raises is conditional, and the condition is not stated. __init__ only checks the shared leading character when start_col > 0 (line 145):

if start_col > 0:
    if len(np.unique(char_array[mask, 0])) > 1:
        raise ValueError(
            "Input array is not compressible — "
            "indices do not share expected prefix"
        )

A root-level construction (start_col == 0) skips it entirely, so as written this over-claims for the root case. "…or, when start_col is nonzero, the masked rows do not share the expected leading character" matches the code.

The first clause ("If mask selects no rows") is unconditional and correct — that is line 139.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Confirmed independently: this example really does fail on main (Failed example: [arr.shape for arr in per_line] / Expected nothing / Got: [(10,), (27,)]), and doctest.testmod on the branch version is TestResults(failed=0, attempted=8). Good fix.

The follow-on worth naming: [(10,), (27,)] is now a pinned behavioural assertion that nothing in CI runs. pyproject.toml's [tool.pytest.ini_options].addopts has no --doctest-modules, testpaths is ["mortie/tests"], and no workflow invokes doctest. If the Rust linestring sampler ever changes its half-cell arc spacing, this becomes wrong documentation silently — the exact failure mode the fix just cleaned up, one layer along.

Two ways out, either acceptable:

(1) add --doctest-modules scoped to the non-test modules, which turns the 37 examples across the four converted modules into a real gate;
(2) leave it, and fold doctest enforcement into the still-open "numpydoc validation in CI" question from the originating issue, so it is a recorded decision rather than an oversight.

Out of scope for a docstring-only PR either way — not asking for it here.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Verified the ImportError commit end to end and it holds:

• exactly one string literal for this message in the tree (morton_index.py line 289; ast.walk over every raise ImportError(...) in that module returns a single literal);
• mortie/pandas.py reaches it via _require_pandas() (line 40), no second copy;
• all three raise paths emit the identical string — test_message_is_identical_on_every_path asserts len(set(msgs)) == 1 and passes;
• docs/morton_index_datatype.md matches character for character — I concatenated the four wrapped comment lines, stripped the # prefixes and the enclosing quotes, and compared to ast.literal_eval of the literal: exact match, 244 chars;
• no stale copy of the old wording anywhere tracked by git (pins pandas, loaded whenever mortie, Install it directly all return zero hits outside .venv/).

The gap: nothing enforces the docs quote. These assertions are substring matches, and the markdown is not checked at all — which is precisely how the previous wording was able to sit in the docs asserting something the code did not mean. A guard that needs no production-code change, reusing the existing helper:

def test_docs_quote_matches_the_message(self):
    out = _run_without_pandas(
        "import mortie\n"
        "try:\n    mortie.MortonIndexArray\n"
        "except ImportError as exc:\n    print(exc)\n"
    )
    doc = (Path(__file__).parents[2] / "docs" / "morton_index_datatype.md").read_text()
    block = re.search(
        r'# "the morton_index ExtensionArray requires pandas.*?"\n', doc, re.S
    ).group(0)
    quoted = " ".join(
        re.sub(r"^#\s?", "", ln).strip() for ln in block.strip().splitlines()
    ).strip('"')
    assert quoted == out.stdout.strip()

Non-blocking, and arguably its own small change rather than this PR's — but it is cheap and it closes the loop the commit was opened to fix.

---

Generated by Claude Code

[espg]

🤖 from Claude

Adversarial self-review ran against the four converted modules and the ImportError commit. It found nothing on the axes the conversion was most at risk on — lost rationale (a token-level audit found only four main words absent, all deliberate summary rewrites; every issue ref #11 #22 #34 #58 #60 #68 #80 #116 #119 #136 and every § link survives), lost or moved comments, the docstring-only claim, examples that don't run, r""" text neutrality, section ordering, and the ImportError commit's single-definition and exact-quote properties. It independently reproduced every gate number in the PR body.

It also confirmed the morton_coverage_moc relocation fixed a live rendering bug rather than a stylistic one: griffe's numpy parser on main emits four returns entries for that function — one real, three bogus, one per line of the multipart paragraph that sat under the Returns header. On the branch it emits one returns plus one raises.

Five diff-scoped findings are addressed in 389ce57:

(1) A Returns guarantee that the code does not make — the medium finding, and the one worth reading. I had propagated morton_polygon's existing Refined prefix-cells (len <= *n_cells*). onto geo_morton_polygon and morton_polygon_from_array while adding their missing return descriptions. It is false. morton_polygon does current = list(roots) and only ever grows that list:

current = list(roots)
count = len(current)
...
while count < n_cells and heap:

so when the trie has more root-level children than the budget, the budget is ignored. Reproduced on a 7-root trie — morton_polygon_from_array(m, n_cells=n) returns 7 cells for every n in 1..7. Roots are expanded, never merged, so len(roots) is a floor. All three functions now say so, and their n_cells parameter lines no longer claim "maximum number of cells in the returned list."

Flagging this as substantively wrong prose, not merely unstructured: the claim predates this PR on morton_polygon (unchanged on main). I corrected it rather than only reporting it because the correction is still docstring-only and this PR is where the sentence got copied twice more — but the behaviour question is yours: is the root-count floor intended, or should morton_polygon reject n_cells < len(roots) instead of silently overshooting? I have not touched the code either way.

(2) morton_coverage_moc's new Raises omitted _prep_rings' ring-count ValueError, which its Parameters now advertises a path to. Added.

(3) mort2healpix stated the same-order precondition in Notes but had no Raises, though it propagates mort2norm's mixed-order ValueError. Verified it does raise (Mixed orders in morton array: [6, 8]) and documented it.

(4) _expansion_efficiency's new node note said cost is "zero for a single child and undefined for a leaf." Both wrong: _compact only branches on two or more distinct characters so the single-child case is unreachable, and a leaf gives cost == -1, a negative float rather than anything undefined. Reworded.

(5) MortonChild's new Raises presented the leading-character check unconditionally; it only fires for start_col > 0, so root construction skips it. Qualified.

Two review suggestions left standing as out of scope, both needing a decision rather than a fix:

• The [(10,), (27,)] output now pinned in linestring_coverage's example is a behavioural assertion nothing executes — there is no --doctest-modules in addopts and no workflow runs doctest. I verified all 37 examples by hand; making that automatic is a pyproject.toml change, which this PR is scoped out of.
• Nothing enforces that docs/morton_index_datatype.md keeps quoting the real ImportError text. A guard reusing the existing _run_without_pandas helper would need no production change. Happy to add it if you want it, but it is a new test rather than a re-pointed assertion, so I left it.

Re-verified after the fixes: pytest 804 passed / 12 skipped (baseline unchanged), ruff check --select D clean on all four files, flake8 clean, mkdocs build --strict exit 0, 37 doctest examples 0 failures, and the docstring-only proof still holds for all four files (AST identical, comment streams 123/17/20/0 unchanged).

---

Generated by Claude Code

[espg]

(1) _normalize_antimeridian_polygon has an unused variable. ruff check mortie/tools.py reports F841 for on_antimeridian at what is now line 975 — assigned from np.sum(...) and never read. It is pre-existing on main and fixing it is a logic change, so it is deliberately untouched here to keep the diff docstring-only. The neighbouring mort2bbox computes the same quantity as a mask and uses it; this one computes a count and drops it, which looks like the vestige of an earlier version of the check. Worth its own issue?

yes, own issue. this too:

(2) tools.py is 1348 lines, over CLAUDE.md §4's ~1000-line guidance. It was already 1315 on main; this PR adds 33 lines of docstring and no code. Flagging rather than splitting, since §4 asks for discussion first and a split here would be churn on top of a docs-only change.

(5) Enforcement is still open from #135 itself — whether to add a numpydoc validation pass to lint.yml, or leave the ruff D rules as the mechanical floor with numpydoc structure as convention. All of mortie/ is converted now, so it is decidable; it is out of scope for this PR.

yes, and it's already tracked as a seperate issue in #140

[espg]

Flagging this as substantively wrong prose, not merely unstructured: the claim predates this PR on morton_polygon (unchanged on main). I corrected it rather than only reporting it because the correction is still docstring-only and this PR is where the sentence got copied twice more — but the behaviour question is yours: is the root-count floor intended, or should morton_polygon reject n_cells < len(roots) instead of silently overshooting? I have not touched the code either way.

yeah, that behavior needs to be fixed. We shouldn't ignore... raising, warning, or pinning to a new threshold would all be better options...

[espg]

Nothing enforces that docs/morton_index_datatype.md keeps quoting the real ImportError text. A guard reusing the existing _run_without_pandas helper would need no production change. Happy to add it if you want it, but it is a new test rather than a re-pointed assertion, so I left it.

seems minor and possibly worth doing...