fix: address quality scorecard findings (#873 #890 #891 #892) - #893
Merged
Conversation
- #891: hoist `numpy` and `_std_is_negligible` imports in `_portfolio_cost.py` to module top (numpy is already a runtime dep; no import cycle), and document each intentionally-deferred accessor import in `data.py` as breaking the data <-> accessors cycle. - #892: update CLAUDE.md template version to v1.2.0 (matches .rhiza/template.lock). - #873: link the existing API Stability / deprecation policy from README. - #890: opt out of the rhiza 1:1 test-layout mirror via [tool.check_test_layout] (enforce=false + reason). Tests are organised by behaviour and per-module coverage is guaranteed by the 100% pytest gate. Requires the upstream rhiza-claude opt-out support (Jebel-Quant/rhiza-claude#51). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses multiple quality scorecard findings by clarifying import structure and architecture intent, updating template-version documentation, improving API stability policy discoverability, and documenting an intentional opt-out from strict test-layout mirroring.
Changes:
- Hoisted function-local imports in
_portfolio_cost.pyand documented intentional deferred accessor imports indata.py. - Linked the API stability/deprecation policy from the README and updated the Rhiza template version reference in
CLAUDE.md. - Added an inert (for now)
[tool.check_test_layout]configuration block to document opting out of 1:1 test-file mirroring.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/jquantstats/data.py | Adds explanatory comments for deferred accessor imports to prevent reintroducing circular-import issues. |
| src/jquantstats/_portfolio_cost.py | Hoists numpy and _std_is_negligible imports to module scope to make dependencies explicit. |
| README.md | Adds a prominent link to the published API stability & deprecation policy. |
| pyproject.toml | Documents opting out of strict test-layout parity via a dedicated tool config table. |
| CLAUDE.md | Updates the advertised Rhiza template version to match the repo’s synced version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+523
to
+525
| # Deferred to break the data <-> accessors import cycle: _plots imports | ||
| # DataLike from this module, so hoisting this to module top re-forms the | ||
| # cycle. Keep the import local. |
This was referenced Jul 17, 2026
Closed
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
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.
Addresses four quality-scorecard issues.
#891 — Hoist function-local imports & document the accessor cycle
_portfolio_cost.py:import numpy as npandfrom ._stats._core import _std_is_negligiblemoved to module top. numpy is already a runtime dep (no lazy benefit) and_stats/_coredoesn't import_portfolio_cost(no cycle) —_stats/_performancealready imports it the same way.data.py: each lazy accessor import (.plots,.stats,.reports,.utils) now carries a one-line comment stating it's intentionally deferred to break thedata <-> accessorscycle, so it isn't "helpfully" hoisted later.#892 — Stale template version in CLAUDE.md
CLAUDE.mdnow states templatev1.2.0, matching.rhiza/template.lock/.rhiza/template.yml.#873 — Semver / deprecation policy
docs/STABILITY.mdalready documents the semver guarantee and deprecation timeline for thePortfolio/Datapublic surface and is linked from the docs nav. This PR adds a reference to it from the README Documentation section so the policy is discoverable from the repo landing page too.#890 — Test-layout parity
Tests here are deliberately organised by behaviour under
tests/test_jquantstats/…, and per-module coverage is guaranteed by the 100% line+branch pytest gate (a stronger guarantee than 1:1 file mirroring). This PR opts out of the strict mirror rule via a documented[tool.check_test_layout]table inpyproject.toml(enforce = false+ areason).Verification
make typecheck— clean (ty + mypy strict).make test— 1215 passed, 100% coverage.make fmt— all pre-commit hooks pass (incl. pyproject validation).🤖 Generated with Claude Code