Skip to content

DM-55799: Freeze masked image schema and check for schema inheritance - #90

Merged
timj merged 3 commits into
mainfrom
tickets/DM-55799
Aug 12, 2026
Merged

DM-55799: Freeze masked image schema and check for schema inheritance#90
timj merged 3 commits into
mainfrom
tickets/DM-55799

Conversation

@timj

@timj timj commented Aug 12, 2026

Copy link
Copy Markdown
Member

We froze cell_coadd 1.0.0 without realizing it depended on masked_image v1.0.0.dev0 because inheritance in python masked the version number.

This PR freezes masked_image but also checks at freeze time that no dev schemas are being included.

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes

cell_coadd 1.0.0 inherits from masked_image and was published, and shipped
in data, while masked_image was still at 1.0.0.dev0. A development schema
may be changed in place freely, so an edit to masked_image would have
rewritten the already-immutable cell_coadd document: the released schema
could change underneath every reference to it.

Freezing masked_image closes that. Its own dependencies -- image, mask and
sky_projection -- are all already at 1.0.0, so it can be finalized without
finalizing anything else first.

cell_coadd's frozen document is byte-identical afterwards, because pydantic
flattens a base class's fields into its subclass and the base's version
never appears in the result. That is also why the problem was not visible in
the published artifact: no $ref and no x-lsst-schema-url records the
inheritance, so nothing that reads the document could see the dependency.

The fixture carries over to the finalized version with its stamp normalized.
It exercises 5 of 38 paths and never holds a sky_projection, and a frozen
fixture cannot be widened without a SCHEMA_VERSION bump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.27027% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.04%. Comparing base (680149a) to head (34a2c56).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ython/lsst/images/serialization/_frozen_schemas.py 89.13% 5 Missing ⚠️
python/lsst/images/_masked_image.py 0.00% 1 Missing ⚠️
tests/cli_schema_doubles.py 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #90      +/-   ##
==========================================
+ Coverage   67.85%   68.04%   +0.18%     
==========================================
  Files         143      145       +2     
  Lines       20740    20876     +136     
==========================================
+ Hits        14074    14205     +131     
- Misses       6666     6671       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Comment thread python/lsst/images/serialization/_frozen_schemas.py Outdated
timj and others added 2 commits August 12, 2026 15:54
A frozen document inlines the content of everything the schema depends on,
including each embedded sub-schema's version in its x-lsst-schema-url. So
finalizing a schema whose dependencies are still in development publishes an
immutable document that those dependencies remain free to change underneath,
and the first time one of them is finalized the containing document changes
and can no longer be written.

write_frozen_schemas now checks this before writing, naming every offending
dependency, and a test asserts that no finalized schema in the package has
one. The check runs when a frozen file is first created rather than on every
run, so an already-committed document is never re-validated.

Inheritance is the route that made this easy to miss. pydantic flattens a
base class's fields into its subclass, so a base contributes its fields to
the frozen document without appearing in it as a $ref or an
x-lsst-schema-url; cell_coadd inherits from masked_image and visit_image
does too. No check that read the document could see the dependency at all,
which is why schema_dependencies walks base classes as well as fields, and
does so transitively -- a dependency reached only through a container, a
union, or another schema counts the same.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two tests in test_cli.py named camera_frame_set because it happened to be a
real schema still in development: one wrote an invalid fixture for it to prove
'fixtures refresh' turns a ValidationError into a clean error rather than a
traceback, the other dirtied its fixture to prove a dirty fixture is detected
on every run without being repaired. Both need refresh to actually process the
fixture, and refresh acts only on a schema still in development, so both
quietly depended on camera_frame_set never being finalized.

That dependency cannot hold. Every schema is finalized during a release, so
there are periods with no development schema in the package at all, and
selecting one dynamically would make these tests skip exactly then. They now
use a double that is never finalized, reached by passing its module as the
CLI's --package, which selects that schema and no other.

Freezing camera_frame_set with the old tests failed both after the ordinary
fixture refresh cascade; with these it leaves the suite green. The validation
test also stopped testing its own subject once the schema was finalized:
refresh skipped the invalid fixture entirely and exited zero, because a .dev
fixture no longer matches a finalized live version.

The double's only field is required, so an empty tree fails to read and the
error path under test is genuinely reached.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@timj
timj force-pushed the tickets/DM-55799 branch from 04deaea to 34a2c56 Compare August 12, 2026 23:06
@timj
timj merged commit afa550f into main Aug 12, 2026
19 checks passed
@timj
timj deleted the tickets/DM-55799 branch August 12, 2026 23:17
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