✨ feat(distances): promote AbstractDistance against ParametricQuantity - #679
Conversation
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds an optional promotion rule so AbstractDistance interoperates consistently with unxts.parametric.ParametricQuantity, fixing operand-order-dependent behavior and wiring the registration behind an optional dependency gate.
Changes:
- Register a Plum promotion rule between
AbstractDistanceandParametricQuantitywhenunxts.parametricis installed. - Introduce a small internal optional-dependency enum (
OptDeps) and bumpoptional-dependenciesfloor to support it safely. - Add test coverage to verify promotion + order-independent arithmetic under the optional install.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/distances/test_parametric_promotion.py | Adds tests exercising the optional registration and ensuring Distance * PQ works. |
| src/coordinax/distances/_src/register_parametric.py | Defines the AbstractDistance ↔ ParametricQuantity promotion rule. |
| src/coordinax/distances/_src/init.py | Conditionally imports the parametric registration based on OptDeps. |
| src/coordinax/_src/optional_deps.py | Adds internal OptDeps enum wrapper around optional-dependencies. |
| pyproject.toml | Raises optional-dependencies minimum version and adds a parametric extra. |
76703bd to
4b4d04c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #679 +/- ##
=======================================
Coverage 96.12% 96.12%
=======================================
Files 254 254
Lines 8179 8182 +3
=======================================
+ Hits 7862 7865 +3
Misses 317 317 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…tity` `ParametricQuantity` is not a `unxt.Q` subclass, so the `AbstractDistance`/`Q` promotion rules in `distances/_src/base.py` never reach it. `Distance(1, "pc") * PQ(1.0, "rad")` dispatches to the `Distance`-returning multiply and raises `Distance must have dimensions length`, while the mirrored `PQ * Distance` returns a `PQ` -- the operand order decides whether the expression works. The rule goes in the existing `register_parametric`, already imported behind `OptDeps.UNXTS_PARAMETRIC.installed`, and spells the type `PQ` to match the `from_` overloads above it -- `PQ is ParametricQuantity`. Adds the matching `parametric` extra, which the module had been relying on the test group to supply. This is the core half of GalacticDynamics#672, which fixed the same defect for `coordinaxs.astro`'s `Parallax` and `DistanceModulus`. Those are `AbstractDistance` subclasses, so this subsumes them; astro's stay, since it must work against a `coordinax` predating this, and plum takes the more specific rule where both apply. Only reachability changes, not values: `PQ * Distance` is `PQ(1., 'pc rad')` before and after, and `Distance * PQ` now returns that instead of raising. `tests/unit/distances`: 66 passed. `packages/coordinaxs.astro`: 406 passed, 2 skipped. With the distribution hidden from `importlib.metadata`, `coordinax.distances` imports without pulling in `unxts.parametric`. pre-commit clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4b4d04c to
09532d8
Compare
What
The core half of #672.
ParametricQuantityis not aunxt.Qsubclass, so theAbstractDistance/Qrules indistances/_src/base.pynever reached it:The operand order decided whether the expression worked. #672 fixed this for
coordinaxs.astro'sParallaxandDistanceModulus;coordinax.distances.Distancewas left with the same defect.How
distances/_src/register_parametric.py— the oneadd_promotion_rule, and the only file importingunxts.parametric.distances/_src/__init__.pybehindOptDeps.UNXTS_PARAMETRIC.installed.coordinax/_src/optional_deps.py— first Python use ofoptional-dependenciesin core, which was already a declared dependency. Floor moves>=0.3.2→>=0.5.0, where members sharing a value no longer silently alias.parametric = ["unxts.parametric>=2.0"]extra.Overlap with #672
Astro's rules are on
Parallax/DistanceModulus, which areAbstractDistancesubclasses, so this rule subsumes them. Theirs stay: astro must work against acoordinaxpredating this, and where both apply plum takes the more specific one. Confirmed the two coexist — all three types give aParametricQuantityin both operand orders.Values are unchanged, only reachability.
PQ * DistanceisPQ(1., 'pc rad')both with and without the new rule; the difference is thatDistance * PQnow returns it instead of raising.Verification
tests/: 2153 passed, 6 skippedpackages/coordinaxs.astro: 398 passed, 2 skipped (coexistence)importlib.metadata,coordinax.distancesimports without pulling inunxts.parametric, and the new tests skip rather than failpre-commit(incl.ty): clean🤖 Generated with Claude Code