From f9dd916933919911df394ddbd1d4f18f971d51d9 Mon Sep 17 00:00:00 2001 From: nstarman Date: Fri, 24 Jul 2026 22:23:29 -0400 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=A9=B9=20fix(v0.24):=20validate=20cha?= =?UTF-8?q?rt=20component=20count;=20drop=20dead=20normalize=5Fvector?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small robustness fixes surfaced by the v0.24 pre-release audit: * charts: concrete `AbstractFixedComponentsChart` subclasses carrying an `AbstractDimensionalFlag` (e.g. `Abstract3D`, n=3) now assert their component count matches the declared dimension at class-creation time, replacing the six `# TODO: add a check it's ND` placeholders in d0-d6 with one central check (the flag records `_chart_ndim`; `__init_subclass__` compares it to the component tuple and raises a clear TypeError). Abstract / variable-n charts are unaffected. * vectors: remove the orphaned `normalize_vector` stub — a `@plum.dispatch.abstract` with no concrete registration, no public export, no callers and no tests. (The audit's third item — a 1D@2D QMatrix matmul path — is dropped: #571 removed the whole `quantity_matrix` module in favor of `unxts.linalg`, so it no longer belongs in coordinax.) Co-Authored-By: Claude Opus 4.8 --- src/coordinax/_src/base/charts.py | 20 +++++++++++++ src/coordinax/_src/charts/d0.py | 2 -- src/coordinax/_src/charts/d1.py | 2 -- src/coordinax/_src/charts/d2.py | 2 -- src/coordinax/_src/charts/d3.py | 2 -- src/coordinax/_src/charts/d4.py | 2 -- src/coordinax/_src/charts/d6.py | 2 -- src/coordinax/vectors/_src/__init__.py | 1 - src/coordinax/vectors/_src/api.py | 13 --------- tests/unit/charts/test_base.py | 40 ++++++++++++++++++++++++++ 10 files changed, 60 insertions(+), 26 deletions(-) delete mode 100644 src/coordinax/vectors/_src/api.py diff --git a/src/coordinax/_src/base/charts.py b/src/coordinax/_src/base/charts.py index 58705cfac..913e41e20 100644 --- a/src/coordinax/_src/base/charts.py +++ b/src/coordinax/_src/base/charts.py @@ -364,6 +364,20 @@ def __init_subclass__(cls, **kw: Any) -> None: cls._coord_dimensions = _get_tuple(args[2]) break + # Check the component count matches the declared dimension flag (if + # this chart mixes in an `AbstractDimensionalFlag` with a fixed `n`). + ndim = getattr(cls, "_chart_ndim", None) + if ( + isinstance(ndim, int) + and hasattr(cls, "_components") + and len(cls._components) != ndim + ): + msg = ( + f"{cls.__name__} is declared {ndim}D but has " + f"{len(cls._components)} components {cls._components}" + ) + raise TypeError(msg) + super().__init_subclass__(**kw) # AbstractChart has. @property @@ -389,9 +403,15 @@ class AbstractDimensionalFlag: """ + #: Declared coordinate dimension of the flag (set when ``n`` is given). + _chart_ndim: ClassVar[int | L["N"]] + def __init_subclass__(cls, n: int | L["N"] | None = None, **kw: Any) -> None: if n is not None: DIMENSIONAL_FLAGS[n] = cls + # Record the declared dimension so concrete fixed-component charts + # can validate their component count against it. + cls._chart_ndim = n # Enforce that this is a subclass of AbstractChart unless it's an # abstract base class (name starts with "Abstract") diff --git a/src/coordinax/_src/charts/d0.py b/src/coordinax/_src/charts/d0.py index ee3a576e2..1f00b243f 100644 --- a/src/coordinax/_src/charts/d0.py +++ b/src/coordinax/_src/charts/d0.py @@ -31,8 +31,6 @@ class Abstract0D(AbstractDimensionalFlag, n=0): A 0D representation has no coordinate component. """ - # TODO: add a check it's 0D - @override def __init_subclass__(cls, n: int | L["N"] | None = None, **kw: Any) -> None: # Enforce that this is a subclass of AbstractChart diff --git a/src/coordinax/_src/charts/d1.py b/src/coordinax/_src/charts/d1.py index 9b7e81bc2..f5270404c 100644 --- a/src/coordinax/_src/charts/d1.py +++ b/src/coordinax/_src/charts/d1.py @@ -49,8 +49,6 @@ class Abstract1D(AbstractDimensionalFlag, n=1): Cartesian $(x)$ or radial $(r)$ coordinates. """ - # TODO: add a check it's 1D - @override def __init_subclass__(cls, n: int | L["N"] | None = None, **kw: Any) -> None: # Enforce that this is a subclass of AbstractChart diff --git a/src/coordinax/_src/charts/d2.py b/src/coordinax/_src/charts/d2.py index 6505200dd..41f0e8144 100644 --- a/src/coordinax/_src/charts/d2.py +++ b/src/coordinax/_src/charts/d2.py @@ -35,8 +35,6 @@ class Abstract2D(AbstractDimensionalFlag, n=2): two angular coordinates but represents a curved surface. """ - # TODO: add a check it's 2D - @override def __init_subclass__(cls, n: int | L["N"] | None = None, **kw: Any) -> None: # Enforce that this is a subclass of AbstractChart diff --git a/src/coordinax/_src/charts/d3.py b/src/coordinax/_src/charts/d3.py index 4ab0ec702..be7e60a76 100644 --- a/src/coordinax/_src/charts/d3.py +++ b/src/coordinax/_src/charts/d3.py @@ -54,8 +54,6 @@ class Abstract3D(AbstractDimensionalFlag, n=3): (cylindrical, spherical), or other three-dimensional manifolds. """ - # TODO: add a check it's 3D - @override def __init_subclass__(cls, n: int | L["N"] | None = None, **kw: Any) -> None: # Enforce that this is a subclass of AbstractChart diff --git a/src/coordinax/_src/charts/d4.py b/src/coordinax/_src/charts/d4.py index 2afb1a6a2..19d203ac6 100644 --- a/src/coordinax/_src/charts/d4.py +++ b/src/coordinax/_src/charts/d4.py @@ -17,8 +17,6 @@ class Abstract4D(AbstractDimensionalFlag, n=4): example is the Minkowski spacetime chart ``(ct, x, y, z)``. """ - # TODO: add a check it's 4D - @override def __init_subclass__(cls, n: int | L["N"] | None = None, **kw: Any) -> None: # Enforce that this is a subclass of AbstractChart diff --git a/src/coordinax/_src/charts/d6.py b/src/coordinax/_src/charts/d6.py index 7030d828c..7b6cc9431 100644 --- a/src/coordinax/_src/charts/d6.py +++ b/src/coordinax/_src/charts/d6.py @@ -28,8 +28,6 @@ class Abstract6D(AbstractDimensionalFlag, n=6): Examples include Cartesian representations in arbitrary dimensions. """ - # TODO: add a check it's 6D - @override def __init_subclass__(cls, n: int | L["N"] | None = None, **kw: Any) -> None: # Enforce that this is a subclass of AbstractChart diff --git a/src/coordinax/vectors/_src/__init__.py b/src/coordinax/vectors/_src/__init__.py index 15c600441..b6dc3203a 100644 --- a/src/coordinax/vectors/_src/__init__.py +++ b/src/coordinax/vectors/_src/__init__.py @@ -1,6 +1,5 @@ """Vectors.""" -from .api import * from .base import * from .bundle import * from .constants import * diff --git a/src/coordinax/vectors/_src/api.py b/src/coordinax/vectors/_src/api.py deleted file mode 100644 index 67c432c2f..000000000 --- a/src/coordinax/vectors/_src/api.py +++ /dev/null @@ -1,13 +0,0 @@ -"""Copyright (c) 2023 coordinax maintainers. All rights reserved.""" - -__all__ = ("normalize_vector",) - -from typing import Any - -import plum - - -@plum.dispatch.abstract -def normalize_vector(x: Any, /) -> Any: - """Return the unit vector.""" - raise NotImplementedError # pragma: no cover diff --git a/tests/unit/charts/test_base.py b/tests/unit/charts/test_base.py index 589877dee..5d2868b82 100644 --- a/tests/unit/charts/test_base.py +++ b/tests/unit/charts/test_base.py @@ -181,3 +181,43 @@ def test_flags_must_subclass_chart(self) -> None: for _flag_cls in cxc.DIMENSIONAL_FLAGS.values(): # All registered flags should have chart subclasses pass # Registration enforces this + + def test_component_count_must_match_declared_dimension(self) -> None: + """A concrete chart's component count must match its dimension flag.""" + import dataclasses + + from typing import Literal + + import jax.tree_util as jtu + + from coordinax._src.base import ( + MT, + AbstractFixedComponentsChart, + chart_dataclass_decorator, + ) + from coordinax._src.charts.d3 import Abstract3D, Cart3D + from coordinax._src.custom_types import Len + from coordinax._src.euclidean.manifold import R3 + + two_keys = tuple[Literal["a"], Literal["b"]] + two_dims = tuple[Len, Len] + + # A 2-component chart declared 3D (via Abstract3D) must be rejected. + with pytest.raises(TypeError, match="declared 3D but has 2 components"): + + @jtu.register_static + @chart_dataclass_decorator + class _Bad3D( + AbstractFixedComponentsChart[MT, two_keys, two_dims], Abstract3D + ): + _: dataclasses.KW_ONLY + M: MT = R3 + + @property + def cartesian(self): + return Cart3D(M=self.M) + + def test_component_count_matches_for_predefined_charts(self) -> None: + """Every predefined chart's ndim matches its component count.""" + assert cxc.cart3d.ndim == len(cxc.cart3d.components) == 3 + assert cxc.polar2d.ndim == len(cxc.polar2d.components) == 2 From 4640546d2fb443e0a016f166c02b2bd83fcad01c Mon Sep 17 00:00:00 2001 From: nstarman Date: Sat, 25 Jul 2026 11:00:32 -0400 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=A7=AA=20test(charts):=20check=20comp?= =?UTF-8?q?onent=20count=20for=20all=20predefined=20charts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Broaden test_component_count_matches_for_predefined_charts to loop over every predefined chart instance (discovered via AbstractChart) instead of just cart3d and polar2d, matching the docstring's "every predefined chart" claim. Co-Authored-By: Claude Opus 4.8 --- tests/unit/charts/test_base.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/unit/charts/test_base.py b/tests/unit/charts/test_base.py index 5d2868b82..96d243a50 100644 --- a/tests/unit/charts/test_base.py +++ b/tests/unit/charts/test_base.py @@ -219,5 +219,12 @@ def cartesian(self): def test_component_count_matches_for_predefined_charts(self) -> None: """Every predefined chart's ndim matches its component count.""" - assert cxc.cart3d.ndim == len(cxc.cart3d.components) == 3 - assert cxc.polar2d.ndim == len(cxc.polar2d.components) == 2 + charts = [ + obj + for name in dir(cxc) + if not name.startswith("_") + and isinstance(obj := getattr(cxc, name), cxc.AbstractChart) + ] + assert charts, "no predefined chart instances discovered" + for chart in charts: + assert chart.ndim == len(chart.components), chart From faa95b7297705356373cdbb7f7bf394ab721162b Mon Sep 17 00:00:00 2001 From: nstarman Date: Sun, 26 Jul 2026 13:48:11 -0400 Subject: [PATCH 3/4] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor(charts):=20si?= =?UTF-8?q?mplify=20chart=20component-count=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the `isinstance`/`hasattr` guards in `AbstractFixedComponentsChart.__init_subclass__` and compare the component count against `_chart_ndim` directly, annotating it as `int | L["N"]`. Signed-off-by: nstarman --- src/coordinax/_src/base/charts.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/coordinax/_src/base/charts.py b/src/coordinax/_src/base/charts.py index 913e41e20..0a4111a83 100644 --- a/src/coordinax/_src/base/charts.py +++ b/src/coordinax/_src/base/charts.py @@ -367,11 +367,7 @@ def __init_subclass__(cls, **kw: Any) -> None: # Check the component count matches the declared dimension flag (if # this chart mixes in an `AbstractDimensionalFlag` with a fixed `n`). ndim = getattr(cls, "_chart_ndim", None) - if ( - isinstance(ndim, int) - and hasattr(cls, "_components") - and len(cls._components) != ndim - ): + if len(getattr(cls, "_components", None)) != ndim: msg = ( f"{cls.__name__} is declared {ndim}D but has " f"{len(cls._components)} components {cls._components}" From 6eca5a14e44a79fd81cef9900a48a85904b360ed Mon Sep 17 00:00:00 2001 From: nstarman Date: Mon, 27 Jul 2026 10:00:07 -0400 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=90=9B=20fix(charts):=20only=20run=20?= =?UTF-8?q?component-count=20check=20for=20fixed=20integer=20dimension=20f?= =?UTF-8?q?lags?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The simplified check ran for every non-abstract fixed-component chart, including `CartND` whose variable dimension flag has `_chart_ndim == "N"` — so `len(components) != "N"` is always true and raised `TypeError` at import (breaking all CI). Restore the guard: run the check only when `_chart_ndim` is a concrete `int` and `_components` is set. Co-Authored-By: Claude Opus 4.8 --- src/coordinax/_src/base/charts.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/coordinax/_src/base/charts.py b/src/coordinax/_src/base/charts.py index 0a4111a83..a1b9022e0 100644 --- a/src/coordinax/_src/base/charts.py +++ b/src/coordinax/_src/base/charts.py @@ -364,10 +364,16 @@ def __init_subclass__(cls, **kw: Any) -> None: cls._coord_dimensions = _get_tuple(args[2]) break - # Check the component count matches the declared dimension flag (if - # this chart mixes in an `AbstractDimensionalFlag` with a fixed `n`). + # Check the component count matches the declared dimension flag, + # but only when the chart mixes in an `AbstractDimensionalFlag` with + # a fixed integer `n` (skip the variable-`n` flag, e.g. `CartND` + # whose `_chart_ndim` is `"N"`, and charts with no flag at all). ndim = getattr(cls, "_chart_ndim", None) - if len(getattr(cls, "_components", None)) != ndim: + if ( + isinstance(ndim, int) + and hasattr(cls, "_components") + and len(cls._components) != ndim + ): msg = ( f"{cls.__name__} is declared {ndim}D but has " f"{len(cls._components)} components {cls._components}"