fix: loud align() error copy for incomparable as-of keys + un-clobberable research shim#128
Open
helloiamvu wants to merge 5 commits into
Open
fix: loud align() error copy for incomparable as-of keys + un-clobberable research shim#128helloiamvu wants to merge 5 commits into
helloiamvu wants to merge 5 commits into
Conversation
…as-of knowledge columns
README-validation find: a BYO spine WITHOUT the obs source's equality-join
key ('date') falls to the as-of path, where merge_asof crashed with a bare
pandas MergeError (ISO-string 'date' vs datetime decision_time). Per the
error-copy contract, _gate_asof_knowledge_dtype now raises a ContractError
naming the source, the column + dtypes, and the concrete fix — with a
distinct message for each shape: missing equality key (add 'date' to the
spine / use weather.days() or mr.spine()), non-datetime knowledge column,
and tz-naive vs tz-aware mixing. The gate only raises where merge_asof
would raise anyway — no previously-working input changes behavior (pinned
parity gate 5/5 byte-green).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Shipped-1.17.0 regression found via README validation: the __getattr__ scrub only runs when a SHIM ACCESS performs the first import of the mostlyright.research submodule. When anything else imports it first (the eager obs() path does 'from mostlyright.research import _resolve_station'), the import system auto-binds the SUBMODULE as the package attribute with no scrub — mostlyright.research(...) then raised TypeError: 'module' object is not callable for the rest of the process (fresh-process repro: eager weather.obs() then mostlyright.research()). Fix: the package's module class now carries a data-descriptor 'research' property (data descriptors outrank the instance __dict__), so the shim stays authoritative regardless of import order; the property setter no-ops the import system's auto-binding. Warning semantics unchanged (once-per-session via _LEGACY_SHIM_WARNED). Regression tests: in-process parent-binding simulation + fresh-subprocess end-to-end repro. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mbol stays patchable Reviewer findings on 275f216/8cecd3d (codex 1 CRITICAL + 2 HIGH; Python Architect 1 HIGH — the Architect independently verified the working-tree predicate rewrite against a live merge_asof dtype matrix): - CRITICAL(codex): the gate required datetime keys, rejecting numeric as-of joins merge_asof supports (BYO int64 decision_time spine worked pre-gate). The predicate is now EXACT-DTYPE-EQUALITY within an as-of-orderable family (numeric | datetime | timedelta) — empirically probed: int64/int64 OK, int64/float64 raises, ns/us raises, UTC/Eastern raises, equal-object raises. Only raises where pandas raises. - HIGH(codex): resolution/timezone mismatches between aware datetimes fell through to raw MergeError — now gated with copy naming both dtypes. - HIGH(codex, accepted trade-off): the property masks 'import mostlyright.research as rm' module semantics — documented in-code: that form appears nowhere in repo/docs; sys.modules/importlib/from-import/ mock.patch dotted targets all preserved (pinned by test); the package attribute has been reserved for the legacy callable since 34-06 (whose byte-identity tests rule out a callable-module design). - HIGH(architect): the unconditional no-op setter swallowed INTENTIONAL writes — monkeypatch.setattr(mostlyright, 'research', stub) silently no-oped and mock.patch teardown crashed on the missing deleter. The setter now stores every write; the getter treats exactly ONE value as non-authoritative (the auto-bound submodule object); a deleter restores cleanly. Regression test covers monkeypatch + mock.patch + del. Fast suite 4625 passed; pinned parity 5/5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tched user overrides
Python Architect iter-2 (verified live): globals().pop('research', None) in
__getattr__'s legacy branch — originally the auto-binding scrub — became
dead weight once the data-descriptor property neutralized the auto-bound
submodule, so the ONLY thing it could still delete was an INTENTIONAL user
override: an active mock.patch('mostlyright.research') was silently
un-patched the moment the code under test touched ANY other legacy symbol
(dataset/Station/CATALOG/live), reverting to the real function mid-test.
The pop is removed (the live scrub stays — live has no property);
test_root_research_symbol_remains_patchable now touches CATALOG + dataset
while the mock is active and asserts the stub survives.
Fast suite 4625 passed; pinned parity 5/5.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
📝 Docs-required check: REMINDER API-surface change without docs and no opt-out — surfacing reminder. API-surface files changed: Docs files changed: Docs surfaces to consider
How to silence this reminder
This check is advisory — it never blocks the merge. |
|
Parity ticket gate: PASSED See |
Member
Author
|
Review loop complete (REVIEW-DISCIPLINE, codex gpt-5.5-high + Python Architect):
Known accepted residual (below gate, both reviewers concur): pathological |
pandas 2 infers ns and pandas 3 infers us from to_datetime, so the implicit spine unit made the mismatch vanish under pd3 (pandas-3-suite DID-NOT-RAISE). Both sides now pin units explicitly; verified under both majors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two post-v1.17.0 correctness fixes, both found while validating README examples against the shipped surface (v1.17.1 candidates).
1.
align(): ContractError instead of raw pandas MergeErrorA BYO spine without the obs source's equality-join key (
date) falls to the as-of path, wheremerge_asofcrashed with a bareMergeError(ISO-stringdatevs datetimedecision_time). Per the error-copy contract,_gate_asof_knowledge_dtypenow raises aContractErrornaming the source, both dtypes, and a concrete fix — with distinct copy for: spine missing the equality key, non-orderable knowledge column, and exact-dtype mismatch (tz/resolution/width). The predicate mirrorsmerge_asofexactly (empirically probed dtype matrix, pinned by tests): it only raises where pandas would raise, so no working input changes behavior — numeric as-of keys keep working.2. Legacy
mostlyright.research()shim: shipped 1.17.0 regressionAny first import of the
mostlyright.researchsubmodule outside the root__getattr__path (the eagerobs()path does this internally) auto-bound the submodule over the legacy callable —mostlyright.research(...)then raisedTypeError: 'module' object is not callablefor the rest of the process. Fixed with a data-descriptor property on the package's module class; intentional writes (monkeypatch/mock.patch) are honored, only the import system's auto-binding is dropped, and the stale__getattr__scrub that could silently un-patch user mocks is removed. Fresh-subprocess end-to-end repro test included.Review loop (REVIEW-DISCIPLINE, codex gpt-5.5-high + Python Architect)
import mostlyright.research as rmbinds the callable; form unused anywhere, package attribute contractually reserved since 34-06).c1c7075, confirmation pending (will be posted here).Gates: fast suite 4625 passed, pinned parity 5/5 byte-green, live monitor unaffected.
🤖 Generated with Claude Code