Skip to content

Remove the ScarletModelData to LsstScarletModelData converter (DM-55150) - #1386

Open
fred3m wants to merge 1 commit into
mainfrom
tickets/DM-55150
Open

Remove the ScarletModelData to LsstScarletModelData converter (DM-55150)#1386
fred3m wants to merge 1 commit into
mainfrom
tickets/DM-55150

Conversation

@fred3m

@fred3m fred3m commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

A base ScarletModelData cannot be promoted to an LsstScarletModelData: the latter requires a single catalog-wide observed PSF and band list, while a ScarletModelData carries them per blend. The conversion never produced a usable model in the science pipelines, so drop the storage-class converters entry (and the now-removed scarlet_model_to_lsst_scarlet_model it referenced, deleted in a companion commit on this ticket).

Old ScarletModelData datasets continue to load at their own storage class.

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes
  • (if changing dimensions.yaml) make a copy of the updated dimensions.yaml in configs/old_dimensions and update the list in doc/lsst.daf.butler/dimensions.rst

@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.89%. Comparing base (78e7f29) to head (eb59e68).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1386      +/-   ##
==========================================
- Coverage   89.90%   89.89%   -0.01%     
==========================================
  Files         373      373              
  Lines       51089    51089              
  Branches     5953     5953              
==========================================
- Hits        45930    45929       -1     
  Misses       3766     3766              
- Partials     1393     1394       +1     

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

@timj

timj commented Aug 12, 2026

Copy link
Copy Markdown
Member

@TallJimbo I am guessing we need to keep this converter around for a while given #1411

@TallJimbo

Copy link
Copy Markdown
Member

I'm not sure; #1411 is about the other direction, which in practice is about supporting new pipelines/tasks on old repos. That's important because most extant repos qualify as "old" - i.e. they have the old storage class for the dataset type where this is used.

So I think this PR is just for supporting old pipelines/tasks (which now just means older software versions) against new repos (dp2* at USDF). And that means it can probably follow a regular-software deprecation cycle.

@TallJimbo

Copy link
Copy Markdown
Member

Oh, no, nevermind: we do need this in the long term, because if #1411 makes a new PipelineTask write the old storage class in an old repo, a consuming new PipelineTask will then need the conversion provided by this PR. Which, I suppose, might not work given the limitations that motivated this PR. So it's possible this whole scheme is doomed to failure, and we will have to do something else like a dataset type rename for backwards compatibility.

A base ScarletModelData cannot be promoted to an LsstScarletModelData: the
latter requires a single catalog-wide observed PSF and band list, while a
ScarletModelData carries them per blend. The conversion never produced a
usable model in the science pipelines, so drop the storage-class
`converters` entry (and the now-removed `scarlet_model_to_lsst_scarlet_model`
it referenced, deleted in a companion commit on this ticket).

Old ScarletModelData datasets continue to load at their own storage class.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@timj
timj force-pushed the tickets/DM-55150 branch from 8703f03 to eb59e68 Compare August 21, 2026 16:19
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.

3 participants