Skip to content

Make Instrument indexable via the na.Indexable mixin#61

Merged
roytsmart merged 3 commits into
mainfrom
feature/select-channel
Jun 9, 2026
Merged

Make Instrument indexable via the na.Indexable mixin#61
roytsmart merged 3 commits into
mainfrom
feature/select-channel

Conversation

@jacobdparker

Copy link
Copy Markdown
Contributor

Summary

Adds a reusable way to reduce a multi-channel esis.optics.Instrument to a single channel.

  • esis.optics.dataclass_select(instance, item) — a recursive helper that indexes every named_arrays.AbstractArray field of a dataclass along a named axis. It recurses into nested dataclasses and into dict/list/tuple containers (e.g. the ruling spacing coefficients dict), and leaves arrays that don't carry the indexed axis untouched (some named-array types such as FunctionArray raise on a missing axis, so the index is filtered to axes each array actually has).
  • AbstractInstrument.select_channel(channel) — a copy-safe method that deep-copies the instrument, drops any cached system, and uses dataclass_select to eliminate the channel axis.

This generalizes the manual per-channel indexing in esis.flights.f1.optics.design_single and the ad-hoc copy.deepcopy loop used to model/fit individual channels, and is broadly useful for single-channel modeling, inversion, and diagnostics.

The pattern follows dataclass_select from the sibling multi-slit-solar-explorer repo, extended to handle dict/list/tuple containers and the missing-axis case.

Tests

esis/optics/_select_test.py:

  • a fast synthetic test of dataclass_select (axis reduction, missing-axis pass-through, dict/nested recursion, non-array fields untouched);
  • a parametrized select_channel test over all four channels covering the dict-stored ruling coefficient, copy-safety (original left unmodified), channel-axis removal, and that the reduced instrument still resolves into an optical system.

Plus test_select_channel added to AbstractTestAbstractInstrument.

pytest esis/optics passes (191 tests); ruff and black clean.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Jun 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

Documentation preview available at https://esis-mission.github.io/esis-pr-previews/61-feature/select-channel
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

@roytsmart

Copy link
Copy Markdown
Member

Cross-referencing a parallel approach to the same problem: sun-data/named-arrays#180 adds a generic na.Indexable mixin whose __getitem__ delegates to a recursive na.getitem() (and shape to a recursive na.shape()), indexing every named-array leaf of a dataclass / dict / list / tuple. It overlaps heavily with dataclass_select here.

One important behavioral difference worth reconciling: dataclass_select filters the index per-leaf to only the axes each array actually has, so a leaf that lacks the indexed axis is left untouched. na.getitem does not do this yet — it passes the full index to every leaf. That's fine for ScalarArray (a missing axis is silently ignored), but FunctionArray raises ValueError: item contains axes {...} that does not exist in {...}. So as-is, na.getitem/Indexable would crash on exactly the missing-axis / FunctionArray case this PR handles.

Worth deciding whether dataclass_select should eventually wrap/defer to na.getitem once that per-leaf axis filtering is upstreamed, to avoid two implementations diverging.

@roytsmart

Copy link
Copy Markdown
Member

Follow-up: the FunctionArray missing-axis regression I mentioned above is now fixed upstream in sun-data/named-arrays#181. FunctionArray.__getitem__ now filters out axes it doesn't have (matching ScalarArray) instead of raising, so indexing a function array by an axis it lacks is a no-op.

With that in, na.getitem handles the missing-axis case on its own, so dataclass_select could eventually drop its per-leaf axis filtering and just defer to na.getitem once #181 lands and named-arrays is bumped here.

@jacobdparker jacobdparker force-pushed the feature/select-channel branch from 1911ef5 to bdc6985 Compare June 8, 2026 17:46
@jacobdparker

Copy link
Copy Markdown
Contributor Author

Reworked per @roytsmart's review: instead of the standalone dataclass_select helper, AbstractInstrument now inherits na.Indexable (named-arrays #180), so it gets a recursive named __getitem__/isel/shape for free, and select_channel(channel) is now just a thin wrapper over self[{self.axis_channel: channel}].

The per-leaf missing-axis filtering the old helper did is no longer needed thanks to #181 (FunctionArray now ignores missing axes), so _select.py is removed entirely. This also makes the capability reusable for any instrument (f1 + f2) and other composite objects.

Since this needs na.Indexable, the dependency pin is bumped to named-arrays~=1.6 (v1.6.0 shipped today and includes #180/#181).

Net diff is now just _instruments.py + its tests + the pin bump.

AbstractInstrument now inherits named_arrays.Indexable, giving every
instrument a recursive named __getitem__/isel/shape that indexes its fields
(rebuilding dataclasses via dataclasses.replace, so the result is a new
object and cached properties such as `system` are dropped).

A single channel can then be selected generically with
instrument.isel(channel=...) (equivalently instrument[{axis_channel: ...}]),
which extends to any future vectorized selection without a bespoke method
per axis.

Requires named-arrays >= 1.6 (na.Indexable, #180/#181); pin bumped accordingly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jacobdparker jacobdparker force-pushed the feature/select-channel branch from bdc6985 to 47abccd Compare June 8, 2026 17:58
@jacobdparker jacobdparker changed the title Add esis.optics.dataclass_select and Instrument.select_channel Make Instrument indexable via the na.Indexable mixin Jun 8, 2026
@jacobdparker

Copy link
Copy Markdown
Contributor Author

Per discussion, dropped select_channel entirely rather than keeping it as a wrapper. Since AbstractInstrument now inherits na.Indexable, a channel is selected generically with instrument.isel(channel=i) (or instrument[{instrument.axis_channel: i}]) — a bespoke per-axis method doesn't scale for a vectorized model.

The ESIS-side change is now a single line (inherit na.Indexable) plus a test_isel verifying that selecting a channel via isel returns a model that still resolves into a valid optical system. Pin remains named-arrays~=1.6.

@roytsmart roytsmart merged commit 7ce5700 into main Jun 9, 2026
13 checks passed
@roytsmart roytsmart deleted the feature/select-channel branch June 9, 2026 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants