Add Fittable/mutable framework, rewrite the Path and Frame registries, require Python 3.11

[markshowalter]

Purpose

This branch introduces a Fittable/mutable framework for in-place parameter fitting, rewrites the Path and Frame registries around it, and fixes the regressions that rewrite introduced. It also drops Python 3.8 through 3.10.

The Fittable work is the motivation: pointing corrections, time shifts, plate scales and orbital elements need to be adjustable in place, with any object that depends on them refreshing itself when they change. Supporting that required reworking how Paths and Frames are registered, cached and linked.

Changes / Implementation Details

Fittable and mutable framework

oops/fittable.py is rebuilt around set_params, refresh and freeze. The new oops/mutable.py propagates staleness through objects that merely contain a Fittable sub-object, tracking state through injected _FITTABLE*/_MUTABLE* attributes and an integer version counter. PathShift, FrameShift, TimeShift and Platescale are new; OffsetFOV, Navigation, Rotation and KeplerPath are converted. The four new classes are marked as untested placeholders.

Path and Frame registry rewrite

Wayframe, Waypoint, AliasPath, AliasFrame and RelativeFrame are gone, replaced by NullFrame/J2000Frame, NullPath/SSBPath, ReversedFrame, RelativePath, ReversedPath and RotatedPath. Registries and the registration hook are private. QuickFrame and QuickPath move into their own modules. A new oops/cache.py provides an LRU cache whose clean_key() makes polymath objects usable as dictionary keys.

Fixes to that rewrite

• mutable._refresh_internal never applied _refresh() on an object's first pass, so the mutable.refresh(self) at the end of every constructor did nothing and derived attributes such as _transform, _times and _pos_x were never created.
• Five Path utility classes cached themselves in Frame._FRAME_CACHE rather than Path._PATH_CACHE, leaving the path cache unused and the frame cache holding Paths. Path._wrt lacked the reversal branch its Frame counterpart has, so linking a root path to one of its descendants recursed until the stack overflowed.
• RelativePath discarded the origin Path it needs for the subtraction, and advertised the frame of the wrong Path.
• LinkedFrame's origin check rejected a null frame origin, which is the ordinary case.
• The "already quick" guards were restored to QuickPath.for_path and QuickFrame.for_frame, and extend() repaired in both -- a dict read as a method, frame keys used in a path class, a stale _steps, arrays whose lengths disagreed, a seam that left the tabulated times non-monotonic, and a tuple passed to a two-argument signature.
• SpicePath.get() ignored origin and frame when asked for the SSB. SPICE accepts several names per body, so the name a caller uses is now registered alongside the canonical one; "GLL" resolves to GALILEO_ORBITER.
• KeplerPath had eleven attribute names left behind by an incomplete privatization.
• os.path.basename() was applied to an FCPath in the Juno hosts, which is not os.PathLike; these now use the FCPath.name property as the other hosts do.

Python version

requires-python moves to >=3.11 and both CI matrices now begin at 3.11, testing 3.11 through 3.13 on Linux, macOS and Windows.

Type of Change

• Bug fix
• New feature
• Breaking change
• Refactor
• Documentation
• Tests
• CI/build

Testing

All three entry points pass:

python -m unittest tests/unittester.py               54 tests   OK
python -m unittest tests/hosts/unittester.py          5 tests   OK
python -m unittest tests/unittester_with_hosts.py    59 tests   OK

The main suite went from 19 failing tests to zero, and the host gold-master suite from four of five failing to zero. The Cassini ISS and Galileo SSI gold masters both compare clean, which exercises observation loading, backplane generation and comparison end to end.

Beyond the suite, the reversal path in Path._wrt was checked against independently computed geometry: earth.wrt(moon) matches both -moon.wrt(earth) and a separately constructed SpicePath('EARTH','MOON') to zero residual across 121 epochs. QuickPath.extend and QuickFrame.extend have no test coverage at all, so they were driven directly and their interpolation checked against the underlying slow path and frame, agreeing to 4e-16 relative and 1e-15 absolute respectively.

Several tests were themselves unsound and were fixed: the SpicePath tests restored Path._USE_QUICKPATHS to True in tearDown when the class default is False, corrupting later tests through the base class, and test_spice_shape never cleared the Path registry, so its custom path ID was ignored whenever an earlier test had already registered VENUS.

Potential Impacts

Breaking, public API. Wayframe, Waypoint, AliasPath, AliasFrame and RelativeFrame no longer exist. Path.as_path(id) now returns the registered Path rather than a zero-position Waypoint; use NullPath for the old behaviour. AliasPath(path, frame) becomes NullPath(path, frame=frame). The registries are private (_PATH_REGISTRY, _FRAME_REGISTRY, _register(), _reset_caches()), the cross-class attributes injected by oops/__init__.py are renamed from XXX_CLASS to _Xxx, quick is keyword-only and defaults to None, the QUICK dictionary keys quickpath_cache/quickframe_cache gain a _size suffix, automatic TEMPORARY_* path IDs are gone (path_id is None when unregistered; use is_registered), and SpicePath.USE_SPICEPATH_SHORTCUTS no longer exists, so there is no longer a way to disable SPICE shortcuts for debugging.

Python support. 3.8, 3.9 and 3.10 are dropped. oops/cache.py uses a match statement, so the floor cannot go back below 3.10 without rework.

Performance. Path._PATH_CACHE was never being written to and so never hit; paths were rebuilt on every call. It now works as intended.

Checklist

• Code follows project style (ruff check, ruff format) — n/a: this repository lints with flake8, not ruff; there is no ruff.toml and no [tool.ruff] in pyproject.toml. Across the 109 modified .py files, flake8 findings fall from 364 on main to 310 here, so the change removes 54 and introduces none.
• Type annotations present and mypy passes — n/a: no mypy configuration exists in the repository, and annotations are confined to fittable.py and mutable.py by design.
• No secrets or credentials committed
• No warnings or errors introduced (CI, linters, type checking, builds) or justified in Notes — see the flake8 counts above.
• Docstrings and Sphinx docs updated (if applicable) — docstrings were updated where behaviour changed, including the Raises: clause of RotatedPath. There is no Sphinx docs tree in this repository.
• No temporary or debug code left in — two stray print statements dumping __dict__ were removed from tests/path/test_spicepath.py.
• Performance impact assessed (see Potential Impacts above)
• Breaking changes flagged in Type of Change above

Notes

Follow-up work, none of it blocking:

• PathShift, FrameShift, Platescale and TimeShift are labelled PLACEHOLDER CODE ... NOT YET TESTED in their own docstrings.
• JunoCam and JIRAM remain out of the test suite by choice. tests/hosts/juno/jiram/__init__.py has been migrated to the current gold-master API and the FCPath crash fixed, so it can be enabled when wanted, but its April 2023 gold masters disagree with this branch on sky angles and ring geometry and would need triage first.
• The Juno gold-master directories need renaming in the resource tree; see Gold-master directory rename below. This is the one change in this PR that git cannot carry.
• run-lint.yml has its pull_request/push triggers commented out and pointing at a master branch that does not exist here, so flake8 has never run in CI.
• tests/hosts/unittester.py covers two of the seven instrument packages; juno, hst, voyager, newhorizons and keck are all commented out.

Gold-master directory rename

Gold masters are stored under a directory named after the module string, used verbatim as a path component (oops/gold_master/__init__.py):

self.gold_dir = GOLD_MASTER_PREFIX / args.module / basename_prefix

The Juno masters were adopted in April 2023, before the host packages were renamed from hosts.* to oops.hosts.*; the Cassini and Galileo masters were re-adopted in December 2023 and already use the new name. So the Juno tests ask for oops.hosts.juno.* and find nothing, reporting No gold master for every backplane.

The resource tree is not a git repository, so this has to be applied by hand wherever the resources live, including the self-hosted CI runners:

cd "${OOPS_GOLD_MASTER_PATH:-$OOPS_RESOURCES/gold_master}"
mv hosts.juno.jiram   oops.hosts.juno.jiram
mv hosts.juno.junocam oops.hosts.juno.junocam

Afterwards all four directories share one convention:

oops.hosts.cassini.iss
oops.hosts.galileo.ssi
oops.hosts.juno.jiram
oops.hosts.juno.junocam

The rename is reversible, and it is a no-op for anyone who does not run the Juno tests, since JunoCam and JIRAM are deliberately excluded from tests/hosts/unittester.py. Note that once the masters are found, the JIRAM comparisons do run but disagree with this branch on sky angles and ring geometry, as noted above.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review available on request

• 🔍 Trigger review

Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment @coderabbitai review to review the latest changes. For a full review, comment @coderabbitai full review.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 08c6cb86-951a-4ad9-939b-c1f48003c8f4

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 67.97960% with 1130 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.91%. Comparing base (ebce0e5) to head (711df7d).
⚠️ Report is 23 commits behind head on main.

Files with missing lines	Patch %	Lines
oops/frame/quickframe.py	63.25%	93 Missing and 22 partials ⚠️
oops/frame/spicetype1frame.py	30.76%	86 Missing and 4 partials ⚠️
oops/path/quickpath.py	60.98%	72 Missing and 15 partials ⚠️
oops/frame/frame_.py	70.52%	72 Missing and 12 partials ⚠️
oops/frame/navigation.py	19.48%	62 Missing ⚠️
oops/path/keplerpath.py	77.05%	46 Missing and 7 partials ⚠️
oops/frame/frameshift.py	31.57%	52 Missing ⚠️
oops/path/pathshift.py	30.13%	51 Missing ⚠️
oops/mutable.py	79.47%	27 Missing and 20 partials ⚠️
oops/frame/rotation.py	47.61%	37 Missing and 7 partials ⚠️
... and 39 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #203      +/-   ##
==========================================
+ Coverage   75.40%   77.91%   +2.51%     
==========================================
  Files         192      202      +10     
  Lines       24094    25198    +1104     
  Branches     2926     2784     -142     
==========================================
+ Hits        18168    19633    +1465     
+ Misses       5088     4707     -381     
- Partials      838      858      +20     

☔ 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.

[rfrenchseti]

Exercised this branch as a drop-in replacement for main (via PYTHONPATH, everything else held fixed). Three things worth flagging, one of them a hard break.

1. oops.hosts.newhorizons.lorri cannot be imported

$ python -c "import oops.hosts.newhorizons.lorri"
  File ".../oops/hosts/newhorizons/lorri.py", line 495, in initialize
    ignore = oops.frame.Cmatrix(flipxyz, lorri_flipped, 'NH_LORRI')
TypeError: Cmatrix.__init__() takes from 2 to 3 positional arguments but 4 were given

Cmatrix.__init__ is (self, cmatrix, reference=None, *, frame_id=None) on this branch, so frame_id is keyword-only, but lorri.py:495 still passes it positionally. Since LORRI.initialize() runs at module scope (line 521), the module is unimportable rather than merely broken at call time — any downstream import of the LORRI host fails outright.

This looks like the one straggler from the keyword-only migration: cassini/iss.py, voyager/iss.py, and juno/jiram/__init__.py all pass frame_id= correctly. It isn't caught by CI because the NewHorizons host tests are commented out in tests/hosts/unittester.py:12. The fix is frame_id='NH_LORRI'.

2. Unregistered frames are retained for the life of the process

Frame._register() inserts into the per-subclass _WAYFRAMES dict unconditionally, and constructing the Transform populates Frame._FRAME_CACHE — both happen whether or not a frame_id was supplied. Both containers are plain dicts, so a process that builds many distinct frames grows without bound:

import numpy as np
from polymath import Matrix3
from oops.frame import Frame
from oops.frame.cmatrix import Cmatrix

rng = np.random.default_rng(0)
for _ in range(10_000):
    Cmatrix(Matrix3(np.linalg.qr(rng.normal(size=(3, 3)))[0]))   # note: no frame_id

print(len(Cmatrix._WAYFRAMES), len(Frame._FRAME_CACHE))

On this branch that prints 10000 20002, and maxrss goes from 142 MB to 165 MB — about 2.3 KB retained per frame. On main (36c1502) the identical loop leaves FRAME_CACHE and WAYFRAME_REGISTRY at 2 entries each; only the temporary-id counter moves.

So it is a slow leak rather than a dramatic one, but it is unbounded and scales with the number of distinct frames a process constructs, which makes it a problem for any long-running or batch-style use. Frame._reset_caches() does clear both dicts, but it also drops every registered frame, so it isn't usable as a periodic trim mid-run.

The branch already seems to carry the intended fix: oops/cache.py defines a bounded Cache (maxsize, LRU eviction once ~10% over), and _register() already routes these keys through Cache.clean_key(). It looks like only the containers themselves were left as plain dicts.

3. Two behavior/API changes that deserve a line in the PR description

Equal-valued unregistered frames now dedup to one object. Cmatrix(m)._wayframe is Cmatrix(m)._wayframe is True here and False on main. Any caller that treats an unregistered frame as private to whatever constructed it now shares one with every other caller holding an equal value. Immutability makes that mostly harmless, but it is observable, and it silently weakens "unregistered" from "isolated" to "not addressable by ID".

Public class attributes were privatized with no aliases: Frame.FRAME_CACHE -> Frame._FRAME_CACHE, Frame.WAYFRAME_REGISTRY -> Frame._FRAME_REGISTRY, and Frame.TEMPORARY_FRAME_ID is gone entirely (unregistered frames no longer consume an id). Downstream code that inspects any of these fails with AttributeError. Presumably intentional, but it is a breaking change and isn't currently called out.

[rfrenchseti]

See earlier comment

[rfrenchseti]

Follow-up to my earlier comment: there is also a significant performance regression on this branch, concentrated in frames that aren't SpiceFrame. Numbers below are this branch (284ed66) against main at 36c1502, same machine, BLAS/OMP pinned to one thread, nothing else running.

Symptom

On a geometry workload that evaluates ring surface intercepts across a full Cassini ISS frame, wall clock goes 25.1 s -> 53.5 s (2.1x). A second workload over the same image set that is dominated by FFT work outside oops is unchanged (33.69 s -> 33.81 s), and a mixed 75-image batch comes out at 1.30x overall — so this is not a flat per-call overhead, it is specific to certain frames.

Where it goes

cProfile on the ring case, main vs this branch:

	main	this branch
cspyce._cspyce0.sxform calls	37 463	4 404 257
time in sxform	0.24 s	23.17 s
ringframe.py:127 transform_at_time calls	2	78
total function calls	23.2M	58.8M

Instrumenting the size of the time array arriving at SpiceFrame.transform_at_time explains the count exactly: a 1 465 696-element array is evaluated 3 times on main and 6 times here, and on this branch roughly three of those evaluations fall through to the per-element loop rather than being served by an interpolating frame. 3 x 1 465 696 = 4.40M, which is the sxform count. On main the equivalent evaluations are served by a QuickFrame.

Proximate cause

Frame._USE_QUICKFRAMES (frame_.py:94) defaults to False, and QuickFrame.for_frame() returns the frame unsubstituted when it is not set:

if not frame._USE_QUICKFRAMES:
    return frame

Across the whole tree only two classes opt in — SpiceFrame._USE_QUICKFRAMES = True (spiceframe.py:24) and SpicePath._USE_QUICKPATHS = True (spicepath.py:23). Every other Frame and Path subclass therefore loses QuickFrame/QuickPath substitution entirely. Wrapping QuickFrame.for_frame and counting outcomes on the ring case confirms it: SpiceFrame is substituted 347 times, and RingFrame is substituted 0 of 57 times while being handed the 1 465 696-element array 5 times.

On main there is no such gate — Frame.quick_frame() is eligible to build a QuickFrame for any frame — which is why the same workload never reaches SPICE per element.

One caveat, since it is the obvious thing to try

Setting RingFrame._USE_QUICKFRAMES = True is not a fix. It does remove the cost (53.5 s -> 1.6 s), but it also changes the answer: a derived image-space quantity in my workload moved by roughly 0.8 px, which is far outside the ~1e-10 agreement the branch otherwise shows against main on the same image. Presumably the default interpolation grid is too coarse for a frame with the ring's rotation rate, so the flag can't simply be switched on per class without also settling the time step. Which frames should be quick-frameable, and on what grid, looks like a design decision for whoever did the rewrite rather than something to patch blindly.

Worth stressing that this is purely a performance issue — apart from the LORRI import break in my earlier comment, results on this branch match main to ~1e-10 on everything I measured.

[markshowalter]

Regarding "2. Unregistered frames are retained for the life of the process", we can look into whether a strict upper limit on the _WAYFRAMES dicts is needed. However, I don't see a real-world situation that needs 10,000 C matrices. If you are iteratively testing C matrices in your navigation procedure, the correct solution is to build a Navigation frame that operates atop your Cmatrix frame. The Navigation frame is Fittable, so if the Observation's frame is a Navigation, you can iteratively apply set_params() to it till you find your best-fit solution. That's how it's supposed to work. There's probably a need to update the Cassini ISS class to allow for such a frame to exist, but in the mean time I think obs.frame = Navigation((0,0), obs.frame) would work. Then any update to the frame would change the pointing without creating a brand new Cmatrix object.

Check out hosts/jwst/nircam for an example of navigation as in input parameter to the constructor. Obviously, having a unified set of host modules is needed so that we don't need to implement the navigation input for every host subclass.

Until we get there, I think doing nothing is a reasonable approach to this issue, sinceit just wastes a bit of core memory, nothing more. If memory really is an issue, we could implement a workaround, such as limiting the size of the Cmatrix._WAYFRAMES cache, but I would prefer not.

BTW, the _WAYFRAMES caches are there to make sure that if you define an identical object multiple times, they are all equivalent. It's important for the __setstate__, __getstate__ mechanism.

[markshowalter]

"1. oops.hosts.newhorizons.lorri cannot be imported" is fixed in this checkin. "3. Two behavior/API changes that deserve a line in the PR description" is addressed by a few words in the PR description.

The performance issue will take a little bit of investigation to determine whether the old or new behavior is correct. Specifically, what should happen is that the SpiceFrame uses a QuickFrame but the RingFrame should not. RingFrame + SpiceFrame should invisibly be implemented as RingFrame + QuickFrame, not as its own unique QuickFrame.

[markshowalter]

Next point about timing. You should be using RingFrame(epoch=0.) for Saturn, because the rotation pole is (essentially) fixed. That means that the RingFrame, which is a "despun" version of the planet's PCK frame, is inertial. So by fixing the epoch at any time, you will be devoting roughly zero time to the evaluation of this frame.

I see the timing issue you raised, and am looking into the best solution. But still, this is time you could be saving.

[jnspitale]

Raising an interaction with #201, following @rfrenchseti's note there (#201 (comment)): as they describe it, the registry rewrite here inserts every frame into a per-subclass _WAYFRAMES table regardless of frame_id, dedups equal-valued unregistered frames to a single shared wayframe, and retains each construction for the life of the process.

Two consequences for Observation.set_cmatrix(frame_id=None) (added in #201):

1. Retention. The frame_id=None path exists so batch consumers (SpinDoctor loading thousands of images, each with its own corrected pointing) get one throwaway frame per observation with no global footprint. If every unregistered frame is retained process-lifetime, that's an unbounded leak proportional to images loaded — independent of the cmatrix API, this affects any code constructing anonymous frames in a loop. If the new oops/cache.py LRU bounds this, that may already be the answer; worth confirming _WAYFRAMES is under it.

2. Dedup. Sharing one wayframe between equal-valued immutable frames doesn't corrupt pointing (re-pointing builds a new frame with different values), but it does falsify Add custom C-matrix support via generic Observation.get_cmatrix/set_cmatrix #201's documented contract that each observation owns its frame. And under the Fittable/mutable framework introduced here, "immutable" may no longer hold: if a shared frame object can later be adjusted in place via set_params, dedup turns per-observation pointing into aliased pointing — which would be a real bug, not just a doc issue.

#201 now pins its contract with a test (test_default_frames_are_distinct_and_stay_out_of_registry: equal-valued C-matrices → distinct frame objects; registry/cache sizes unchanged by a default load), so whichever branch merges second will see a concrete failure rather than a silent semantic change. Flagging it now so the reconciliation can be a design decision here rather than a rebase surprise.