Skip to content

✨ feat(astro): gate unxts.parametric registrations on it being installed - #672

Merged
nstarman merged 4 commits into
GalacticDynamics:mainfrom
nstarman:feat/astro-optional-parametric
Aug 7, 2026
Merged

✨ feat(astro): gate unxts.parametric registrations on it being installed#672
nstarman merged 4 commits into
GalacticDynamics:mainfrom
nstarman:feat/astro-optional-parametric

Conversation

@nstarman

@nstarman nstarman commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

unxts.parametric is not a coordinaxs.astro dependency, but its ParametricQuantity is not a unxt.Q subclass either, so the AbstractDistance/Q promotion rules in coordinax.distances never reached it. The result depended on operand order:

Parallax(1, "mas") * PQ(1.0, "rad")   # ValueError: Parallax must have angular dimensions
PQ(1.0, "rad") * Parallax(1, "mas")   # PQ['solid angle'](..., unit='mas rad')

Same for DistanceModulus.

What this does

Adds the two missing promotion rules, in their own _src/register_parametric.py so the optional import is confined to one file, imported from _src/__init__ behind OptDeps.UNXTS_PARAMETRIC.installed.

OptDeps follows the optional-dependencies idiom unxt itself uses. It requires >=0.4.1: earlier versions keyed enum members on the installed version, so any two members sharing one silently collapsed into a single member reporting the wrong package's state — which caught every pair of uninstalled dependencies, and the co-released unxts.* packages (unxts.api, unxts.hypothesis and unxts.parametric are all 2.0.0). That is fixed in 0.4.1 (GalacticDynamics/optional_dependencies#57), so members can be added freely here.

unxts.parametric is declared as a [parametric] extra; the tests skip at module level on the same OptDeps check, so they run where the extra is installed and skip — not fail — where it is not.

Verification

  • Both paths exercised: with the distribution hidden from importlib.metadata, coordinaxs.astro imports without pulling in unxts.parametric at all.
  • Multi-member safety confirmed against the real packages: three unxts.* members at 2.0.0 plus two uninstalled ones all stay distinct.
  • packages/coordinaxs.astro: 398 passed, 2 skipped. pre-commit clean.

Note

coordinax.distances.Distance has the identical gap (Distance * PQ raises the same way). A single add_promotion_rule(AbstractDistance, ParametricQuantity, ...) in core would close it for all three types, but core cannot import an optional package that only astro declares — it would need the same OptDeps treatment there. Left out of scope; happy to follow up.

🤖 Generated with Claude Code

nstarman and others added 2 commits August 6, 2026 20:32
…alled

`unxts.parametric` is not a `coordinaxs.astro` dependency, but its
`ParametricQuantity` is not a `unxt.Q` subclass either, so the
`AbstractDistance`/`Q` promotion rules in `coordinax.distances` never reached
it. `Parallax(1, "mas") * PQ(1.0, "rad")` dispatched to the `Parallax`-
returning multiply and raised `Parallax must have angular dimensions`, while
the mirrored `PQ * Parallax` returned a `PQ` -- the operand order decided
whether the expression worked. Same for `DistanceModulus`.

Adds the two missing promotion rules, in their own
`_src/register_parametric.py` so the optional import is confined to one file,
imported from `_src/__init__` behind `OptDeps.UNXTS_PARAMETRIC.installed`.
`OptDeps` follows the `optional-dependencies` idiom unxt itself uses, with one
caveat carried into its docstring: `OptionalDependencyEnum` keys members on
the installed *version*, and the co-released `unxts.*` packages usually share
one (`unxts.api`, `unxts.hypothesis` and `unxts.parametric` are all 2.0.0), so
a second `unxts.*` member would silently collapse into an alias of the first.

`unxts.parametric` is declared as a `[parametric]` extra; the tests skip at
module level on the same `OptDeps` check, so they run where the extra is
installed and are skipped, not failed, where it is not.

Verified both paths: with the distribution hidden from `importlib.metadata`,
`coordinaxs.astro` imports without pulling in `unxts.parametric` at all.

Note: `coordinax.distances.Distance` has the identical gap, which a single
`add_promotion_rule(AbstractDistance, ParametricQuantity, ...)` in core would
close for all three types. Left alone here -- core cannot import an optional
package that only astro declares.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `OptDeps` enum carried a warning not to add a second `unxts.*` member:
`OptionalDependencyEnum` keyed members on the installed *version*, and any two
sharing one collapsed into a single `enum.Enum` member -- silently, reporting
the wrong package's state. That caught every pair of uninstalled dependencies
and the co-released `unxts.*` packages, which usually share a version
(`unxts.api`, `unxts.hypothesis` and `unxts.parametric` are all 2.0.0 here).

0.4.1 keys members so they stay distinct, so the restriction is gone and the
docstring now says members can be added freely. Verified against the real
packages: three `unxts.*` members at 2.0.0 plus two uninstalled ones all stay
separate, and `.value` is still the `Version`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 7, 2026 00:36
@github-actions github-actions Bot added 🔧 Add / update configuration Add or update configuration files. ✨ Introduce new features Introduce new features. ⬆️ Upgrade dependencies Upgrade dependencies. labels Aug 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes operand-order–dependent promotion behavior between coordinaxs.astro distance-like quantities (Parallax, DistanceModulus) and unxts.parametric.ParametricQuantity by adding explicit Plum promotion rules, while ensuring the registrations only load when unxts.parametric is actually installed.

Changes:

  • Add OptDeps (based on optional-dependencies>=0.4.1) and gate unxts.parametric-specific registrations behind OptDeps.UNXTS_PARAMETRIC.installed.
  • Register missing plum.add_promotion_rule promotions so Parallax/DistanceModulus interactions with ParametricQuantity reliably degrade to ParametricQuantity (order-independent).
  • Add unit tests that skip when unxts.parametric isn’t installed, and add a [parametric] extra to coordinaxs.astro.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
uv.lock Adds optional-dependencies to coordinaxs-astro, adds parametric extra (via unxts-parametric), and bumps optional-dependencies to 0.4.1.
pyproject.toml Ensures workspace test group includes unxts.parametric so the gated registrations are exercised in CI when available.
packages/coordinaxs.astro/pyproject.toml Adds hard dependency on optional-dependencies>=0.4.1 and declares [parametric] extra for unxts.parametric.
packages/coordinaxs.astro/src/coordinaxs/astro/_src/init.py Conditionally imports the side-effect registration module when unxts.parametric is installed.
packages/coordinaxs.astro/src/coordinaxs/astro/_src/optional_deps.py Introduces OptDeps enum for optional dependency checks.
packages/coordinaxs.astro/src/coordinaxs/astro/_src/register_parametric.py Adds the two missing promotion rules to ParametricQuantity.
packages/coordinaxs.astro/tests/unit/distances/test_parametric_promotion.py Adds regression tests for order-independent promotion and preserves existing degrade-to-u.Q behavior.

Moves the floor from 0.4.1 to the mainline release. Both carry the fix that
keeps enum members distinct -- 0.4.1 is the 0.4.x maintenance line (which also
still supports Python 3.9), 0.5.0 is `main` -- so this picks the branch
`coordinaxs.astro` should actually track.

Needed `uv lock --refresh`: 0.5.0 published minutes before this, and uv's
cached index listing still topped out at 0.4.1, so resolution failed on the
Python 3.14 split. `--refresh-package optional-dependencies` was not enough.

Re-verified on 0.5.0: three `unxts.*` members at 2.0.0 plus two uninstalled
ones stay distinct, and `packages/coordinaxs.astro` is 398 passed, 2 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 7, 2026 00:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

@nstarman nstarman added this to the v0.24.0 milestone Aug 7, 2026
The why was written once in the commits and PR body, then two or three more
times in the source. Trimmed to one copy each:

- `OptDeps`: dropped seven lines of `optional-dependencies` release history.
  The pyproject floor enforces it; a reader adding a member needs one fact.
- `register_parametric`: module docstring restated the gate two lines away in
  `_src/__init__`; the block comment was a fourth copy of the operand-order
  failure story. Kept only why the core rules miss `ParametricQuantity`.
- Both pyproject comments, and the test docstring that repeated the same
  narrative a fifth time.

Also drops `# type: ignore[misc]` from `OptDeps`: no mypy session exists in the
noxfile, and `ty` -- which `nox -s lint` does run -- passes without it. The
`pylint` disable stays; pylint is only commented out of `lint` pending a
cleanup, and it does flag this as `E0244` when run.

398 passed, 2 skipped; pre-commit and `ty` clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 7, 2026 00:53
@github-actions github-actions Bot added the 🎨 Improve code structure / format Improve structure / format of the code. label Aug 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

packages/coordinaxs.astro/src/coordinaxs/astro/_src/init.py:16

  • The conditional from .register_parametric import * is used only for its import side effects, and register_parametric.__all__ is empty, so the wildcard import never actually exports anything. This is confusing (and keeps a * import in the module) when a plain module import would be clearer and make the intent explicit.
if OptDeps.UNXTS_PARAMETRIC.installed:
    from .register_parametric import *

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.57%. Comparing base (f1eb540) to head (30a18e1).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #672   +/-   ##
=======================================
  Coverage   95.57%   95.57%           
=======================================
  Files         247      249    +2     
  Lines        8039     8052   +13     
=======================================
+ Hits         7683     7696   +13     
  Misses        356      356           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nstarman
nstarman merged commit e0f05d1 into GalacticDynamics:main Aug 7, 2026
18 checks passed
@nstarman
nstarman deleted the feat/astro-optional-parametric branch August 7, 2026 01:33
nstarman added a commit to nstarman/coordinax that referenced this pull request Aug 8, 2026
…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`; its docstring widens from "`from_`
overloads" to cover both registrations, so an optional dependency stays
confined to one gated file. 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/`: 2253 passed, 7 skipped. `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>
nstarman added a commit to nstarman/coordinax that referenced this pull request Aug 8, 2026
…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>
nstarman added a commit that referenced this pull request Aug 8, 2026
…tity` (#679)

`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 #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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔧 Add / update configuration Add or update configuration files. 🎨 Improve code structure / format Improve structure / format of the code. ✨ Introduce new features Introduce new features. ⬆️ Upgrade dependencies Upgrade dependencies.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants