Skip to content

feat: add InstrumentCapabilities models (§A.1 of duration-estimates plan) - #864

Merged
thusser merged 2 commits into
developfrom
instrument-capabilities-models
Sep 2, 2026
Merged

feat: add InstrumentCapabilities models (§A.1 of duration-estimates plan)#864
thusser merged 2 commits into
developfrom
instrument-capabilities-models

Conversation

@thusser

@thusser thusser commented Sep 2, 2026

Copy link
Copy Markdown
Member

Summary

  • First slice of specs/plans/2026-09-01-instrument-capability-duration-estimates.md (§A.1): new pyobs/robotic/instruments.py with plain pydantic models mirroring pyobs-portal's InstrumentSerializer JSON shape (post-v0.18.19 #139/v0.18.20 #140/pyobs-portal#142module_name lives on CameraCapability/TelescopeCapability/DomeCapability/FilterWheelCapability, not on Instrument itself).
  • InstrumentCapabilities flattens the nested portal response into direct camera()/telescope()/dome()/filter_wheel() lookups by module_name, plus by_camera_code() for the fleet-wide-ID case — no two-step "resolve Instrument, then search its nested list" indirection, since each script only ever needs one device's row.
  • No Django import — this only ever deserializes JSON the portal API already returns.
  • Follows the existing pyobs.utils.serialization.BaseModel convention already used for external-API models (pyobs/robotic/storage/lco/_portal.py's Lco* classes).

Not in this PR

§A.2–A.8 (TaskData field, TaskArchive/PortalTaskArchive wiring, Task.estimate_duration()/TaskScheduler.schedule() threading, and the 5 leaf scripts) are follow-up work, tracked in the plan doc.

Test plan

  • pytest tests/robotic/test_instruments.py — 9/9 passing, including a round-trip test against a real InstrumentSerializer dump from a live pyobs-portal instance (not hand-guessed field names)
  • pytest tests/robotic/ — 451/451 passing, no regressions
  • ruff check / black --check / pyrefly check — clean

🤖 Generated with Claude Code

https://claude.ai/code/session_019XoNCe7YyJELc8xup1zVdE

thusser and others added 2 commits September 2, 2026 20:36
…lan)

New pyobs/robotic/instruments.py: plain pydantic models mirroring
pyobs-portal's InstrumentSerializer JSON shape (post-#139/#140/#142 --
module_name lives on CameraCapability/TelescopeCapability/DomeCapability/
FilterWheelCapability, not on Instrument itself), plus InstrumentCapabilities,
a module-name-keyed view over a GET /api/instruments/ response.

Flattens the nested portal response into direct camera()/telescope()/dome()/
filter_wheel() lookups by module_name at parse time, plus by_camera_code()
for the fleet-wide-ID case -- scripts only ever need one device's row, never
"the instrument" as a concept, so no two-step resolve-then-search indirection.

No Django import; this only ever deserializes JSON the portal API already
returns. Round-trip test fixture is a real InstrumentSerializer dump from a
live pyobs-portal instance, not hand-guessed field names.

First slice of specs/plans/2026-09-01-instrument-capability-duration-estimates.md
(§A.1) -- TaskData/TaskArchive/PortalTaskArchive/scheduler wiring and the 5
leaf scripts (§A.2-A.8) are follow-up work.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019XoNCe7YyJELc8xup1zVdE

@thusser thusser left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: approve — clean, faithful §A.1 slice

I cross-checked this against the actual pyobs-portal code in the sibling repo (post-#140/#142 develop), not just the PR text. No blockers; a few notes below are forward-looking, mostly for the later §A.4–A.8 slices.

Verified

  • Schema fidelity is exact. Field-by-field comparison against pyobs_portal/instruments/serializers.py (current develop): every serializer field maps one-to-one onto the models in pyobs/robotic/instruments.py with correct nullability and defaults — including name: str = "" matching CharField(blank=True), mount_type default, empty-list defaults, nullable telescope/dome, and nullable FilterWheelCapability.module_name. The fixture's timestamps (2026-09-02T18:34:16Z, minutes before this PR opened) corroborate the "live dump, not hand-guessed" provenance.
  • Flat lookup keys can't collide. The portal DB enforces unique=True on module_name per device table and on code (instruments/models.py), so the last-wins behavior in InstrumentCapabilities.__init__ is safe today — including cross-type name sharing, which lands in separate dicts.
  • Tooling all green: 9/9 new tests pass; ruff, black --check, pyrefly clean on both files. The only failures in the wider tests/robotic/ run (446 passed, 4 failed) are environmental — astropy IERS table download blocked by a read-only ~/.astropy cache — and fail identically on develop in this sandbox. Unrelated to the PR.

Notes (none blocking)

  1. extra="forbid" makes parsing hard-fail on portal payload drift — worth deciding now, since §A.4/§A.8 build on it. These models inherit BaseModel's extra="forbid" (#762 convention), so any field the portal adds or renames — a real risk given #139/#140/#142 just reshaped this exact payload — raises ValidationError for the whole response (checked: an extra id key fails with extra_forbidden). That conflicts with the plan's "degrade to None, never raise" principle, and under §A.4's design a first-parse failure leaves capabilities None forever (the stale-cache fallback only helps after a first success). These models are a pure mirror of another repo's wire format — unlike pyobs' own config models where forbid catches typos — so consider extra="ignore" here, or explicitly make §A.4 treat a ValidationError as a fetch failure. Field sets match exactly today, so this is forward-looking only.
  2. Pagination contract for the future fetcher. GET /api/instruments/ is DRF-paginated (PageNumberPagination, PAGE_SIZE=100 in portal settings), so from_api_response's documented "caller unwraps results" contract would silently truncate any fleet with >100 instruments. Fine for real sites; flag for §A.4 or set pagination_class = None on the portal view.
  3. Nit — test style: setup_method is the only xunit-style hook in tests/robotic/; the rest of the suite uses plain functions/fixtures. Cosmetic only.
  4. Nit — updated_at: str: fine as-is (nothing in pyobs-core compares these timestamps; staleness is the portal's marker endpoint), but a one-line "why str, not datetime" comment would preempt the obvious question.

Summary

Models mirror the serializer exactly, the flattening matches §A.1 (including by_camera_code and skipping None-module filter wheels per pyobs-portal#142), docstrings are clear about motivation and contracts, and tests cover the interesting edges (None wheel not indexed, cross-type negative lookup, multi-instrument aggregation). Merge as-is; carry notes 1–2 into the §A.2–A.8 follow-up work.

@thusser
thusser merged commit 08d6f2e into develop Sep 2, 2026
4 checks passed
@thusser
thusser deleted the instrument-capabilities-models branch September 2, 2026 18:43
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.

1 participant