Background
src/picmaker/instruments/__init__.py declares three hand-curated lists:
VICAR_INSTRUMENTS = [cassini, galileo, voyager]
FITS_INSTRUMENTS = [hst, nh]
ALL_INSTRUMENTS = [cassini, voyager, galileo, hst, nh]
Adding a new instrument requires updating all three lists (or two, if it only handles one format). The lists are independently mutable, which makes it possible for them to drift apart — e.g., a new instrument added to ALL_INSTRUMENTS but forgotten in FITS_INSTRUMENTS would silently fail to detect FITS files for that instrument.
See CODEBASE_CRITIQUE.md §1 (Structure and layout, "Finding (Medium) — instruments/__init__.py ships three explicit instrument lists").
Proposed approach
Option A — probe-based derivation. Drop the explicit VICAR_INSTRUMENTS / FITS_INSTRUMENTS lists; iterate ALL_INSTRUMENTS and let detect_vicar / detect_fits short-circuit on instruments that don't handle that format (they already return None unconditionally). This is the minimal change and adds zero new API surface; the cost is calling a no-op detect_* for every probe (negligible — six instruments today).
# instruments/__init__.py
ALL_INSTRUMENTS = [cassini, voyager, galileo, hst, nh]
# VICAR_INSTRUMENTS / FITS_INSTRUMENTS removed.
# io.py reader cascade
for instrument in instruments.ALL_INSTRUMENTS:
filter_info = instrument.detect_vicar(vic) # returns None for non-VICAR
if filter_info is not None:
return (array3d, False, filter_info)
Option B — declarative flags on the instrument module. Each instrument module declares SUPPORTS_VICAR: bool and SUPPORTS_FITS: bool module-level constants; the package derives the filtered lists from those flags:
# instruments/cassini.py
SUPPORTS_VICAR = True
SUPPORTS_FITS = False
# instruments/__init__.py
ALL_INSTRUMENTS = [cassini, voyager, galileo, hst, nh]
VICAR_INSTRUMENTS = [i for i in ALL_INSTRUMENTS if i.SUPPORTS_VICAR]
FITS_INSTRUMENTS = [i for i in ALL_INSTRUMENTS if i.SUPPORTS_FITS]
Option B keeps the named filtered lists (better for discoverability), at the cost of every instrument module needing the two flags (vs. just letting detect_vicar/detect_fits return None).
Recommended
Option A — the detect_* predicates are already the single source of truth (e.g., cassini.detect_fits is unconditionally None). Storing the same information in a second place (VICAR_INSTRUMENTS) is what allows drift. The minor perf cost of calling no-op detect_fits once per cascade per instrument is invisible (~µs).
Acceptance criteria
Related
CODEBASE_CRITIQUE.md §1 — original finding.
Background
src/picmaker/instruments/__init__.pydeclares three hand-curated lists:Adding a new instrument requires updating all three lists (or two, if it only handles one format). The lists are independently mutable, which makes it possible for them to drift apart — e.g., a new instrument added to
ALL_INSTRUMENTSbut forgotten inFITS_INSTRUMENTSwould silently fail to detect FITS files for that instrument.See
CODEBASE_CRITIQUE.md§1 (Structure and layout, "Finding (Medium) —instruments/__init__.pyships three explicit instrument lists").Proposed approach
Option A — probe-based derivation. Drop the explicit
VICAR_INSTRUMENTS/FITS_INSTRUMENTSlists; iterateALL_INSTRUMENTSand letdetect_vicar/detect_fitsshort-circuit on instruments that don't handle that format (they already returnNoneunconditionally). This is the minimal change and adds zero new API surface; the cost is calling a no-opdetect_*for every probe (negligible — six instruments today).Option B — declarative flags on the instrument module. Each instrument module declares
SUPPORTS_VICAR: boolandSUPPORTS_FITS: boolmodule-level constants; the package derives the filtered lists from those flags:Option B keeps the named filtered lists (better for discoverability), at the cost of every instrument module needing the two flags (vs. just letting
detect_vicar/detect_fitsreturnNone).Recommended
Option A — the
detect_*predicates are already the single source of truth (e.g.,cassini.detect_fitsis unconditionallyNone). Storing the same information in a second place (VICAR_INSTRUMENTS) is what allows drift. The minor perf cost of calling no-opdetect_fitsonce per cascade per instrument is invisible (~µs).Acceptance criteria
VICAR_INSTRUMENTS/FITS_INSTRUMENTSremoved fromsrc/picmaker/instruments/__init__.py.src/picmaker/io.pyiteratesinstruments.ALL_INSTRUMENTSfor both VICAR and FITS probes.ALL_INSTRUMENTS) plus the new module file.Related
CODEBASE_CRITIQUE.md§1 — original finding.