✨ feat(fmt): a string-formatting engine unifying repr, str, format, and the IPython reprs - #855
Conversation
|
@adrn the goal isn't to make a package called |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new unxt.fmt formatting engine that unifies how repr, str, format, and IPython rich representations are produced, enabling new types to participate by registering a single pparts implementation. It also updates unit system rendering to ensure repr round-trips via eval(repr(usys)) == usys, and aligns documentation/tests with the new output.
Changes:
- Added
unxt.fmt(public) andunxt._src.fmt(implementation) providing a parts-based formatting pipeline for text/HTML/LaTeX plusFORMAT_PRESETSfor f-string specs (e.g.:compact,:mul,:latex). - Reworked unit system rendering via a shared
AbstractUnitSystem.__pdoc__, updated dataclassrepr=Falsefor all unit-system shapes, and added tests forreprround-tripping and readablestr. - Routed Quantity IPython markup reprs (
_repr_html_,_repr_latex_) through the new engine and registeredUnitsMatrixparts to preserve structure and prevent LaTeX slicing regressions.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_unitsystems.py | Adds property-style tests ensuring unit-system repr round-trips and str/dims formatting behaves as intended. |
| tests/unit/test_quantity_printing.py | Adds tests covering format presets and the __pdoc__ custom-hook chaining behavior. |
| tests/unit/test_fmt.py | New focused test suite for unxt.fmt presets, markup escaping, layout behavior, extensibility, and JIT interactions. |
| src/unxt/fmt.py | New public module re-exporting the formatting engine API. |
| src/unxt/_src/unitsystems/flags.py | Updates doctest outputs for unit-system rendering changes. |
| src/unxt/_src/unitsystems/core.py | Updates doctests and ensures dynamic unit-system dataclasses don’t shadow base rendering (repr=False). |
| src/unxt/_src/unitsystems/builtin.py | Switches built-in unit-system dataclasses to repr=False; adds DimensionlessUnitSystem.__pdoc__. |
| src/unxt/_src/unitsystems/base.py | Adds round-tripping unit-string logic and centralizes unit-system rendering in __pdoc__; updates __repr__, __str__, __format__. |
| src/unxt/_src/quantity/mixins.py | Routes IPython HTML/LaTeX repr methods through the new formatting engine. |
| src/unxt/_src/quantity/base.py | Fixes __pdoc__ custom hook clobbering by chaining hooks; routes __format__ through pspec; adds _chain_custom. |
| src/unxt/_src/fmt.py | New core formatting engine: parts tree model, doc/markup consumers, preset table, and fallback behavior. |
| src/unxt/init.py | Exposes fmt at the package top level. |
| README.md | Updates unit-system rendering examples to match new repr. |
| packages/unxts.linalg/tests/test_printing.py | Adds regression tests ensuring QuantityMatrix markup output is correct (esp. LaTeX). |
| packages/unxts.linalg/src/unxts/linalg/_src/_units_matrix.py | Registers UnitsMatrix with unxt.fmt.pparts to render structured unit tuples correctly. |
| packages/unxts.interop.gala/docs/index.md | Updates doctest output for unit-system rendering. |
| packages/unxts.interop.gala/docs/guide.md | Updates doctest output for unit-system rendering. |
| packages/unxts.interop.gala/docs/api.md | Updates doctest output for unit-system rendering. |
| docs/interop/dataclassish.md | Updates example output to reflect new str rendering of unit systems. |
| docs/index.md | Updates doctest outputs for unit systems to match new repr. |
| docs/guides/units_and_systems.md | Updates doctest outputs for unit systems to match new repr. |
| docs/api/index.md | Adds fmt to the API docs toctree. |
| docs/api/fmt.md | New API docs page for unxt.fmt. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #855 +/- ##
==========================================
+ Coverage 99.82% 99.85% +0.03%
==========================================
Files 84 86 +2
Lines 3985 4130 +145
Branches 311 333 +22
==========================================
+ Hits 3978 4124 +146
Misses 3 3
+ Partials 4 3 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
galax declares `unxt>=1.11.2` with no upper bound, but has never been resolved or tested against unxt 2.x -- the lock has only ever held a 1.x. Today nothing declares that boundary: `uv` considers unxt 2.0.0 during resolution (it is compatible on `requires-python`) and backtracks to 1.11.x for transitive reasons. That is incidental, not a guarantee, and it disappears the moment the surrounding constraints shift. Make the tested boundary explicit. The resolved version is unchanged -- 1.11.2, as before -- so this is metadata only; `uv.lock` records the new specifier and nothing else. There is a concrete change coming that this guards against. unxt is making unit system `repr`/`str` round-trippable (GalacticDynamics/unxt#855), which rewrites the `LTMAUnitSystem( length=Unit("kpc"), ...)` form that 32 doctest lines here pin, 18 of them in `_interop/galax_interop_gala/potential.py` and 12 in `..._galpy/potential.py`. Those updates belong with the port to unxt 2.x, not ahead of it. Lift this cap in the PR that does that port. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ab86974 to
c77438d
Compare
Addresses Copilot's review on GalacticDynamics#855: `pformat_doc` was imported from `wadler_lindig._wadler_lindig`, a private module path that can move between releases. wadler-lindig only lays out a *document* through that private function -- its public surface takes objects, not docs. Handing `wadler_lindig.pformat` an object whose `__pdoc__` returns the document reaches the same code through the public API, and is verified to produce byte-identical output. Exposed as `doc_to_str`, so the two test modules that reached for the private path use it too, leaving no private wadler-lindig import anywhere in the repo. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses review on GalacticDynamics#855. **`repr` of a named realization is now its name.** `planck` was rendering as four seventeen-digit constants -- correct, since `to_string()` truncates past six significant figures and the truncated form does not reconstruct, but unreadable. Every realization is registered, and `unitsystem('planck')` reconstructs exactly, so it is both shorter *and* still round-trippable: before: unitsystem('1.6162550244237053e-35 m', '2.1764343427178984e-08 kg', ...) after: unitsystem('planck') The full-precision spelling survives only for an *ad hoc* system carrying a lossy scale and no registered name -- `unitsystem(DynamicalSimUSysFlag, ...)` is the one case in the docs. Dropping it there instead would have silently broken `eval(repr(x)) == x`, so it stays; a test now pins that guarantee across the realizations, a single-unit system, a flag-built system, and an extension. The reverse mapping is declared in `base` and populated by `realizations`, which already imports `base` -- the consumer registers into the lower layer, matching the direction the engine now follows. **Units always use the list form**, `unitsystem(['m', 's', 'kg', 'rad'])`. A lone string argument is looked up as a *system* name rather than a unit, so a one-unit system needed the list anyway; using it at every arity removes a special case that only appeared at `n == 1`. The `DynamicalSimUSysFlag` doctests keep their scale behind an ellipsis: it is derived from `G`, so pinning full precision would break on a CODATA update. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses a suppressed Copilot comment on GalacticDynamics#855: the block still showed `unitsystem('kpc', 'Myr', ...)`, the varargs spelling this PR replaced with the list form. No test run could have caught it. The example carried `# doctest: +SKIP`, so it was never executed -- which is also how it drifted in the first place, and why regenerating expected output from pytest's reported values missed it. Rather than just correct the text, drop the `+SKIP` and hide the derived mass unit behind an ellipsis. It is computed from `G`, which is what made the example unstable enough to skip; an ellipsis keeps it robust to a CODATA update while letting the doctest actually verify the shape, so it cannot silently drift again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
c2d8bac to
804f3c4
Compare
A shared engine behind `repr`, `str`, `format` and the IPython
representations, so they agree on *what an object is made of* and a new type
can join in by registering one function. Nothing calls it yet; this commit is
purely additive.
An object registers `pparts`, returning a tree of roled fragments. Two
consumers turn that tree into output:
- `parts_to_doc` builds a wadler-lindig document, so plain-text rendering gets
layout, line breaking and nesting for free and composes inside a larger
document. The engine *feeds* wadler-lindig rather than replacing it.
- `parts_to_markup` flattens to HTML or LaTeX. Those need no layout, and
pushing markup through wadler-lindig would corrupt its width accounting --
it measures `len(ansi_strip(text))`, so every `<span>` bills as visible
columns.
Three axes extend independently: a new type registers `pparts`; a new role
needs nothing, since a fragment carries its own plain-text fallback; a new
markup is one `MARKUPS` row.
Details that are load-bearing, each covered by a test:
- Composites nest child parts (`PGroup`) rather than splicing them flat or
embedding rendered strings. Splicing loses the grouping boundary, and a
wadler-lindig group is all-or-nothing, so every break point would break
together; embedding a string applies the markup wrapper once per child and
emits invalid nested `$...$`.
- A separator's visible text must survive a break -- `BreakDoc` shows its text
only in horizontal mode -- and only a separator with trailing space offers a
break, or adjacent separators produce a blank line.
- Escaping defaults to on, so a fragment carrying real markup must say so;
escaping unxt's own LaTeX would turn `\mathrm` into `\textbackslash`.
- A dimensionless unit's emptiness is decided on its plain string: its LaTeX
form is `$\mathrm{}$`, truthy once stripped, and would emit a phantom unit.
- `pparts` has an `Any` fallback. This is a display path, so an unregistered
type must degrade rather than poison an entire object's repr in a notebook.
`plum`'s global dispatcher keys on the bare `__name__` in one shared
namespace, so two libraries defining `pparts` would silently merge their
method tables; the engine uses a module-local `Dispatcher`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`_repr_html_` and `_repr_latex_` built their strings by hand, never touching
`__pdoc__` or anything else. They now go through `unxt.fmt`, so the structure
decision -- value, separator, unit -- lives in one place.
Fixes a live corruption. `_repr_latex_` sliced `[1:-1]` off the unit's
representation, assuming astropy's `$...$` wrapping. `UnitsMatrix` has no
`_repr_latex_`, so the fallback `__repr__` was sliced and lost its first and
last characters:
before: '$[1.,~2.,~3.] \; nitsMatrix("(m, s, kg)"$'
after: '$[1.,~2.,~3.] \; (m, s, kg)$'
Nothing covered this, so it ships with a regression test.
`unxts.linalg` registers `pparts` for `UnitsMatrix` in the same change rather
than a later one: with the wiring but without the registration, a
`QuantityMatrix`'s LaTeX would go from corrupted-but-returning to raising --
worse than the bug being fixed.
Two intended output changes. The unit in `_repr_html_` was rendered by a
`getattr(unit, "_repr_html_", unit.__repr__)` fallback; astropy units have no
`_repr_html_`, so that fallback always fired and emitted `<span>Unit("m")</span>`
where `<span>m</span>` was meant. And these reprs now render a jit-traced value
as a shape/dtype summary instead of raising `TracerArrayConversionError`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…Dynamics#683) `f"{q:compact}"`, `f"{q:mul}"`, `f"{q:latex}"` and the rest of `unxt.fmt.FORMAT_PRESETS` now work as format specs. Existing behaviour is unchanged: an empty spec is `str`, and any non-preset spec still goes to the value with the unit appended, astropy-style. The preset lookup runs *before* the value-spec branch. That ordering is mandatory rather than stylistic -- handing a non-empty spec straight to the value raises for a tracer and for any non-scalar array, so a preset checked second would be unreachable under `jax.jit` and for every array quantity. Grammar is exact-match, with no `:` reserved and no combining. A spec may use `:` as its fill character, so `f"{q::>12.2f}"` must keep working; the accompanying test pins it. All eight preset names were checked to be currently *invalid* specs for float/int/complex/float32, so registering them is strictly additive. An unknown spec now raises `ValueError` naming the type and listing the presets, instead of NumPy's `unsupported format string passed to numpy.ndarray.__format__`. A *valid* spec that fails only because the value is a non-0-d array keeps its original `TypeError` -- calling that an invalid spec would be a lie, and downstream `except TypeError` handlers depend on it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The same constructor returned two different repr styles depending on which
shape it landed on:
unitsystem("m", "s", "kg", "radian") -> unitsystem(m, s, kg, rad)
unitsystem(["km"]) -> LengthUnitSystem(length=Unit("km"))
unitsystem("planck") -> LengthMassTimeTemperatureUnitSystem(...)
Four classes hand-rolled a `__repr__`; every other shape -- including every
dynamically-created one -- fell through to the dataclass default. And because
`AbstractUnitSystem` had no `__pdoc__` at all, *nested* wadler-lindig
renderings used the dataclass form regardless, so a unit system inside a larger
repr disagreed with its own `repr()`.
One `__pdoc__` on the base now renders all of them, with `show_units=False`
selecting the dimension-name form that `__str__` prints.
The load-bearing part is `repr=False` on the five `@dataclass` decorators in
`builtin.py` and on the `make_dataclass` in `core.py`. Without it this change
is a silent no-op: `@dataclass` plants a generated `__repr__` in every
subclass's `__dict__`, so a base-class `__repr__` is unreachable and deleting
the hand-rolled ones merely swaps them for generated ones. Every doctest would
have kept passing.
`DimensionlessUnitSystem` keeps an override, converted from `__repr__`/`__str__`
to `__pdoc__` so nested renderings agree. Its generic form would be
`unitsystem()` -- correct, and it does round-trip, since `unitsystem()` returns
that system -- but the class name reads better for the one system with no units
to show.
23 doctest lines move from the dataclass form to the compact one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BREAKING CHANGE: `repr(unitsystem(...))` now quotes its units, `str` takes the
unquoted compact form, and the dimension-name rendering moves from `__str__` to
the `{usys:dims}` format preset.
repr: unitsystem('kpc', 'Myr', 'solMass', 'rad') (was: unitsystem(kpc, ...))
str: unitsystem(kpc, Myr, solMass, rad) (was: LTMAUnitSystem(length, ...))
dims: LTMAUnitSystem(length, time, mass, angle) (via f"{usys:dims}")
`repr` should reconstruct the object, and now does: `eval(repr(usys)) == usys`
holds for all nine realizations, checked by a test that discovers them
dynamically rather than from a hand-written list -- `hep` and `geometrized` are
easy to leave off one, and both exercise the fallback below.
Two details are load-bearing for that, both found by testing the property
rather than assuming it:
- A single-unit system emits the list form, `unitsystem(['km'])`. A lone string
argument is looked up as a *system* name, so the bare form raises instead of
rebuilding.
- A scaled unit falls back to the full-precision scale when `to_string()` does
not reparse equal. `to_string()` truncates past six significant figures,
which loses `planck`, `atomic`, `hep` and `geometrized`. Python's float
`repr` is already the shortest round-tripping string. A companion test pins
that the fallback stays *rare* -- `galactic`, `solarsystem`, `si` and `cgs`
must render with named units and no numeric scale -- so a regression in the
"shortest" half cannot silently make every repr long while the round-trip
test still passes.
The dimension-name form had no consumers and only existed because someone once
wrote a `__str__`; the engine gives it somewhere better to live.
Doctests that hid CODATA constants behind `...` keep doing so, so an astropy
constant update cannot break them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fills the gaps a coverage run surfaced after the rebase onto GalacticDynamics#847's 100%-coverage baseline. The `custom=` chain had no regression test at all -- the fix that motivated it was only ever checked by hand. Both directions are now pinned: a caller's hook wins under every `short_arrays` mode, and rendering is unchanged when no hook is passed. Also covers the two remaining branches in the engine's array summary: a weak dtype keeps its `weak_` prefix, and a `StaticValue` (not an array, so the kind hook declines it) unwraps to `f64[2]` without the `(numpy)` suffix. One path is only reachable by calling `__pdoc__` directly: everything routed through `wadler_lindig.pformat` carries wl's own `_none` default, so `custom` is never actually absent from the forwarded kwargs. `unxt._src.fmt` is now at 100%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lint BREAKING CHANGE: `unxt.fmt` is now `unxt._fmt` and is no longer exported from `unxt` or documented under `docs/api/`. The engine is still settling and has not yet been exercised by a downstream package, so it stays out of the public surface until its shape is proven. Also fixes the `Format` CI job, which runs pylint (exit code 24 = convention + refactor): - Two `import-outside-toplevel`. The lazy imports are load-bearing -- `unxt._src.fmt` imports `unxt._src.quantity.base`, which imports `mixins`, so a module-scope import cycles -- so they get a module-level disable with the reason spelled out, matching the precedent already in `quantity/base.py`. - A real `duplicate-code`: the engine had its own copy of `custom_pdoc_no_kind`, widened to `numpy.ndarray`. Widen the original instead and import it. One hook, one definition -- and `__pdoc__`'s `short_arrays=True` path now drops the `(numpy)` kind suffix consistently with the JAX case, which is what the widening was for. Nothing asserted the old form. - The shim's `__all__` necessarily mirrors the implementation's, so it carries the same `duplicate-code` disable `unxt/__init__.py` already uses for its re-export surface. `unxts.linalg` now reaches across into `unxt._fmt` to register `pparts` for `UnitsMatrix`. That is a private cross-package import; it is tolerable while both live in this workspace, and is the first thing to revisit when the engine goes public. pylint is back to 10.00/10. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `unxts.interop.gala` src doctests still expected the pre-flip unquoted `repr`. Missed locally because the noxfile's path list for that package is only its `tests/`, while CI additionally runs `packages/unxts.interop.gala/src` under `--doctest-modules` -- a separate invocation, because the leaf directory `unxts/interop/gala` shadows the installed `gala` and sybil mis-imports it. Verified with CI's exact command, and swept every other package's src suite the same way (`unxts-hypothesis`, `unxts-interop-xarray` under `--doctest-modules`; `unxts-api`, `unxts-parametric`, `unxts-linalg` under sybil). All clean. Also refreshes the `+SKIP` example in `docs/index.md`, which is never executed but would have shown the old form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`codecov/patch` failed on a single partial branch: `_chain_custom`'s
`if caller is None` guard. Both arcs are genuinely reachable -- a direct
`__pdoc__` call omits `custom` entirely, while anything routed through
`wadler_lindig.pformat` carries wl's own default -- but coverage would not
report the pair as complete, and a guard that needs a test per arc to prove
itself is not worth keeping.
Remove it. `kwargs.get("custom", _no_custom)` supplies a decline-everything
hook when none was passed, so the chain is unconditional and there is no branch
to be partial about. `_no_custom` mirrors what wadler-lindig's own default does.
No behaviour change; the existing tests for both directions still pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e engine The engine had the layering backwards. `unxt._src.fmt` imported `unxt._src.quantity.base` for `AbstractQuantity` and `custom_pdoc_no_kind`, and `unxt.units` for `AbstractUnit`, then registered `pparts` for both. That made the engine depend on its own consumers, and the resulting cycle is what forced `quantity/base.py`, `quantity/mixins.py` and `unitsystems/base.py` to import it lazily inside methods, each with a `pylint: disable` explaining the workaround. Dependencies now point inward. `unxt._src.fmt` imports no `unxt` module at all -- only stdlib, jax, numpy, wadler-lindig and plum -- and every domain-specific rendering is registered *into* it: - `unxt._src.units` registers `pparts` for `AbstractUnit` - `unxt._src.quantity.base` registers `pparts` and `pspec_fallback` for `AbstractQuantity` - `unxts.linalg` already registered `pparts` for `UnitsMatrix`, and needed no change -- which is the evidence that the extension seam was the right shape; it was only the built-in types that were wired the wrong way round. Consequences: - All three lazy imports become ordinary module-scope imports, and both `import-outside-toplevel` disables are deleted. - `custom_pdoc_no_kind` and `custom_pdoc_noarray` move to the engine. They are generic array-to-doc helpers with no quantity knowledge, and the engine needs one of them; `__pdoc__` imports them back. - `_value_str` becomes public `value_str`, and `_bad_spec` becomes `bad_spec`, since the quantity registration in `base.py` now calls both. A downstream composite that factors units out of its components needs `value_str` for the same reason. No behaviour change: same output, same tests, 10.00/10 pylint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses Copilot's review on GalacticDynamics#855: `pformat_doc` was imported from `wadler_lindig._wadler_lindig`, a private module path that can move between releases. wadler-lindig only lays out a *document* through that private function -- its public surface takes objects, not docs. Handing `wadler_lindig.pformat` an object whose `__pdoc__` returns the document reaches the same code through the public API, and is verified to produce byte-identical output. Exposed as `doc_to_str`, so the two test modules that reached for the private path use it too, leaving no private wadler-lindig import anywhere in the repo. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses review on GalacticDynamics#855. **`repr` of a named realization is now its name.** `planck` was rendering as four seventeen-digit constants -- correct, since `to_string()` truncates past six significant figures and the truncated form does not reconstruct, but unreadable. Every realization is registered, and `unitsystem('planck')` reconstructs exactly, so it is both shorter *and* still round-trippable: before: unitsystem('1.6162550244237053e-35 m', '2.1764343427178984e-08 kg', ...) after: unitsystem('planck') The full-precision spelling survives only for an *ad hoc* system carrying a lossy scale and no registered name -- `unitsystem(DynamicalSimUSysFlag, ...)` is the one case in the docs. Dropping it there instead would have silently broken `eval(repr(x)) == x`, so it stays; a test now pins that guarantee across the realizations, a single-unit system, a flag-built system, and an extension. The reverse mapping is declared in `base` and populated by `realizations`, which already imports `base` -- the consumer registers into the lower layer, matching the direction the engine now follows. **Units always use the list form**, `unitsystem(['m', 's', 'kg', 'rad'])`. A lone string argument is looked up as a *system* name rather than a unit, so a one-unit system needed the list anyway; using it at every arity removes a special case that only appeared at `n == 1`. The `DynamicalSimUSysFlag` doctests keep their scale behind an ellipsis: it is derived from `G`, so pinning full precision would break on a CODATA update. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three doctests in `unxts.interop.gala` still showed the positional spelling and broke on the switch to `unitsystem(['km', 's', 'solMass', 'rad'])`. They were missed because I had been running `packages/unxts.interop.gala/tests` locally, while CI runs `packages/unxts.interop.gala/src` -- the interop packages are exercised through their *src doctests*, per `_parse_pytest_paths` in the noxfile. Re-ran every package through `nox -s "pytest(package=...)"`, which is what CI actually invokes, rather than guessing at paths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses a suppressed Copilot comment on GalacticDynamics#855: the block still showed `unitsystem('kpc', 'Myr', ...)`, the varargs spelling this PR replaced with the list form. No test run could have caught it. The example carried `# doctest: +SKIP`, so it was never executed -- which is also how it drifted in the first place, and why regenerating expected output from pytest's reported values missed it. Rather than just correct the text, drop the `+SKIP` and hide the derived mass unit behind an ellipsis. It is computed from `G`, which is what made the example unstable enough to skip; an ellipsis keeps it robust to a CODATA update while letting the doctest actually verify the shape, so it cannot silently drift again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Carries across the hardening from GalacticDynamics#870, which this PR supersedes. `pparts(AbstractUnit)` sliced `to_string("latex")[1:-1]` unconditionally. That is safe for astropy units, which always wrap -- but it is the same shape as the bug GalacticDynamics#870 fixed, and it would corrupt any fragment arriving unwrapped: `\mathrm{m}` becomes `mathrm{m`. Since this engine is the extension point that downstream packages register into, the assumption is worth removing before someone else's unit type inherits it. `unwrap_math` lives in the engine rather than at the call site: it is generic markup handling with no unit knowledge, and it is the kind of rule a second markup would otherwise re-derive. The length guard is load-bearing -- a lone `"$"` satisfies both `startswith` and `endswith` and would otherwise be sliced away entirely. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Answers the review question about whether `_roundtrip_unit_str` was necessary.
Measured: it fired for 2 of 13 systems, both *ad hoc* -- one built from
`DynamicalSimUSysFlag`, one extending `planck` with a velocity. Every
conventional realization spells its units exactly, and every measured-constant
one is caught by the registered-name path added alongside it.
For those two it bought exact reconstruction at the price of
unitsystem(['m', 'kg', '122404.43065054427 s'])
which is the wall of digits the review objected to in the first place, just
relocated. They now show the short units and do not reconstruct exactly:
unitsystem(['m', 'kg', '122404 s'])
matching what `main` already prints for that example today.
So `repr` round-trips for every *named* system -- the guarantee the test pins,
discovered dynamically so a new realization is covered without anyone
remembering. It stops there, deliberately, and the test now says so rather
than claiming it universally.
`_exact_unit_strs` stays: it is what routes a lossy system to its name instead
of printing truncated units, and so is what keeps the conventional systems
showing their units rather than falling back to a bare name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
8c10508 to
affbc94
Compare
|
@adrn, if you like the look of this, I'll actually factor it out into a small companion utility library so that we can use this unified engine for I might try to get some stuff upstreamed into wadler_lindig, particularly a class so invisible elements like |
Closes #683.
Adds
unxt._fmt: a shared engine behindrepr,str,format, and the IPythonrepresentations, so they agree on what an object is made of and a new type can
join in by registering one function.
How it works
A type registers
pparts, returning a tree of roled fragments. Two consumersturn that tree into output:
parts_to_docbuilds a wadler-lindig document, so plain-text renderinggets layout, line breaking, and nesting for free and composes inside a larger
document. The engine feeds wadler-lindig rather than replacing it.
parts_to_markupflattens to HTML or LaTeX. Those need no layout, andpushing markup through wadler-lindig would corrupt its width accounting — it
measures
len(ansi_strip(text)), so every<span>bills as visible columns(measured: 1.2×–2.2× inflation, misbreaking at the default width).
Three axes extend independently, each covered by a test: a new type
registers
pparts; a new role needs nothing, since a fragment carries itsown plain-text fallback; a new markup is one
MARKUPSrow.The module is private for now (
unxt._fmt, not exported fromunxt, notin
docs/api/). The engine is still settling and has not been exercised by adownstream package yet, so it stays off the public surface until its shape is
proven.
unxts.linalgreaches across into it to registerppartsforUnitsMatrix— a private cross-package import, tolerable while both live inthis workspace, and the first thing to revisit when it goes public.
Prototyped against the cases this has to grow into — an
Anglecarrying abranch cut, a quantity with uncertainty, and a coordinax-shaped vector holding
components plus a frame and a chart.
Breaking change
repr/strof unit systems (see the💥 boomcommit):reprunitsystem(kpc, ...)orLengthUnitSystem(length=Unit("km"))unitsystem('kpc', 'Myr', 'solMass', 'rad')strLTMAUnitSystem(length, time, mass, angle)unitsystem(kpc, Myr, solMass, rad)strf"{usys:dims}"reprnow reconstructs the object:eval(repr(usys)) == usysholds for allnine realizations. Two details are load-bearing, both found by testing the
property rather than assuming it — a single-unit system needs the list form
(
unitsystem(['km']), since a lone string is a system name), and themeasured-constant realizations need a full-precision scale fallback because
to_string()truncates past six significant figures.Downstream: galax has ~34 doctest lines that change (all nested
LTMAUnitSystem( length=Unit("kpc"), ...)inside potential reprs). It pinsunxt>=1.10.3with no upper bound, so its CI goes red on release regardless ofmerge order — that needs either a
unxt<Xcap added before release or a shortred window. A companion PR is not yet prepared. coordinax and potamides are
unaffected; neither asserts on unit-system rendering.
Bugs found here, fixed separately
Two of the bugs originally described as "fixed in passing" here have since
landed as their own PRs and are no longer part of this diff — this branch is
rebased on top of both:
__pdoc__discarded a caller'scustom=hook — fixed in 🐛 fix(quantity): stop__pdoc__discarding a caller'scustom=hook #869(merged). It assigned
kwargs["custom"], andwadler_lindig.pdocalwaysputs
customin the kwargs it forwards, so the caller's was dropped on thedefault path.
_repr_latex_corruptedQuantityMatrix— fixed in 🐛 fix(quantity): stop_repr_latex_corrupting units without_repr_latex_#870 (merged). Itsliced
[1:-1]assuming astropy's$...$wrapping, so aUnitsMatrix(which has no
_repr_latex_) lost its first and last characters:'$[1.,~2.,~3.] \; nitsMatrix("(m, s, kg)"$'. This PR's ownppartsengine had the same assumption baked into its
AbstractUnitrendering(
to_string("latex")[1:-1]), so the🐛 fix(fmt): strip $...$ only when the LaTeX fragment has themcommit here carries the same hardening acrossinto the new code path, via a shared
unwrap_mathhelper.One does not have a life outside this PR, since it's only meaningful once the
new
reprexists:repr=Falseon the unit-system dataclasses — without it the wholeunit-system change is a silent no-op, since
@dataclassplants a generated__repr__in every subclass that shadows the base's, and every doctestkeeps passing.
Notes for review
Sixteen commits after the rebase onto main (down from the original
seventeen — the standalone
__pdoc__fix dropped out as a no-op once #869merged); the
💥 boomcommit is the point of no return and could be split outif you'd rather land the rest first.
Two design decisions worth a look, both argued in the commit messages: the
engine uses a module-local
plum.Dispatcher(the global one keys on bare__name__, so two libraries definingppartswould silently merge methodtables), and composites nest child parts rather than splicing them flat
(a wadler-lindig group is all-or-nothing, so splicing makes every break point
break together).
unxt._src.fmtis at 100% coverage, and pylint is at 10.00/10.Milestone: set to
v2.1.0to match the two most recent PRs, but thiscarries a breaking change — if you want strict semver that wants a major
milestone instead.