Annotate return types of PowerSeriesRing, LaurentPolynomialRing and QuotientRing (follow-up to #42670) - #42675
Open
starnotes-xj wants to merge 4 commits into
Open
Conversation
Contributor
Author
|
@cxzhong @tobiasdiez — this is the second follow-up to #42670 (annotates three more Sage constructor functions: PowerSeriesRing, LaurentPolynomialRing, QuotientRing). Could you take a look? The CI runs for the current head Note: the docker "Build & Test" shard reddening is the unrelated ecl.pyx abort tracked in #42680 (it reproduces on branches that do not touch libs/ecl.pyx). |
…uotientRing
Static type checkers cannot infer the return type of these module-level
constructor functions. Add return annotations using the common base of
the concrete implementations (verified against Sage 10.9):
- PowerSeriesRing -> PowerSeriesRing_generic (covers PowerSeriesRing_domain,
PowerSeriesRing_over_field and MPowerSeriesRing_generic)
- LaurentPolynomialRing -> LaurentPolynomialRing_generic (covers the
univariate and multivariate implementations)
- QuotientRing -> QuotientRing_generic (covers the generic and
polynomial quotient implementations)
TESTS blocks assert the annotation against the runtime type for the
common univariate and multivariate cases. Follow-up to sagemath#42670.
PowerSeriesRing and QuotientRing are annotated with their generic base class, which is defined further down in the same module. On Python < 3.14 annotations are evaluated eagerly at def time, so the module fails to import on 3.13 (the fork doc build hit 'NameError: name QuotientRing_generic is not defined'). Add 'from __future__ import annotations' so the names resolve lazily. LaurentPolynomialRing needs no change: its generic base lives in laurent_polynomial_ring_base and is imported at the top.
Use local forward references where classes are defined later, keep the Laurent and power-series common bases, and broaden QuotientRing to the Parent contract used by all supported branches. Extend doctests across annotation resolution and representative factory paths. Constraint: QuotientRing returns unchanged parents and multiple specialized quotient implementations\nRejected: QuotientRing_generic | excludes noncommutative, principal, pbori, and zero-ideal paths\nRejected: Module-wide postponed annotations | changes unrelated runtime introspection semantics\nConfidence: high\nScope-risk: moderate\nReversibility: clean\nDirective: Preserve local forward references unless module-wide annotation semantics are intentionally reviewed\nTested: python -m py_compile on all three modules; git diff --check\nNot-tested: Sage doctests require the configured Sage build environment
starnotes-xj
force-pushed
the
annotate-factory-function-returns
branch
from
August 23, 2026 18:19
fb1a079 to
0bc02dc
Compare
starnotes-xj
marked this pull request as ready for review
August 23, 2026 18:20
The regression doctest uses Parent explicitly, so import it in the doctest namespace instead of relying on module globals. Constraint: Sage doctests do not expose module-level imports automatically\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nTested: git diff --check\nNot-tested: Sage doctest runtime
|
Documentation preview for this PR (built with commit 663f741; changes) is ready! 🎉 |
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.
Follow-up to #42670: this PR annotates three constructor functions whose return types static type checkers cannot infer.
PowerSeriesRingreturnsPowerSeriesRing_generic, the common base of its univariate and multivariate implementations.LaurentPolynomialRingreturnsLaurentPolynomialRing_generic, the common base of its univariate and multivariate implementations.QuotientRingreturnsParent: unlike the narrower generic quotient class, this accurately covers all supported branches, including ordinary and principal quotients, noncommutative quotients, Boolean-polynomial implementations, integer quotients and the zero-ideal path that returns the original parent.Forward references are quoted locally where the implementation class is defined later in the module, avoiding import-time
NameErrorwithout changing annotation evaluation semantics for unrelated functions.Regression doctests cover representative runtime branches and
typing.get_type_hintsresolution. No runtime behavior changes; these are annotations only.