DM-55542: Fixes for dimension universe compatibility#1397
Open
timj wants to merge 4 commits into
Open
Conversation
In python mode the model_dump can return set() which can then not be dumped back to json using the DimensionConfig.dump method.
3 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1397 +/- ##
=======================================
Coverage 89.84% 89.85%
=======================================
Files 373 373
Lines 50857 50945 +88
Branches 5924 5928 +4
=======================================
+ Hits 45695 45777 +82
- Misses 3772 3778 +6
Partials 1390 1390 ☔ View full report in Codecov by Harness. |
Rebuilding a dataset type or ref in a different but compatible dimension universe was implemented independently in the transfer_from import path and the hybrid test registry, and downstream code needs the same operation. Consolidate it in DatasetType.conform_to and DatasetRef.conform_to, and migrate those call sites. The consolidated check is stricter than the import path's original: in addition to requiring a matching namespace and identical dimension names, the required/implied split of the conformed dimension group must match, since the required dimensions determine data ID keys and dataset table columns. Co-Authored-By: Claude Fable 5 <[email protected]>
HybridButlerRegistry.registerDatasetType must only convert dataset types from the remote butler's universe instance; other universes are passed through unchanged so that the direct registry's strict universe check still applies. Conforming everything broke the registry test that verifies registration rejects mismatched universes. Also test conform_to against real historical universes loaded from the old_dimensions configuration files, in both a compatible case (universe 7) and one where the dimension group has since gained a member (universe 2). Co-Authored-By: Claude Fable 5 <[email protected]>
Comparing against the default universe would make the tests sensitive to future universe versions; compare frozen universe 7 (or 2) against frozen universe 8 instead. Co-Authored-By: Claude Fable 5 <[email protected]>
timj
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In python mode the model_dump can return set() which can then not be dumped back to json using the DimensionConfig.dump method.
Also unify DatasetType conformance across universes (needed by lsst/pipe_base#572)
Checklist
doc/changesconfigs/old_dimensionsand update the list indoc/lsst.daf.butler/dimensions.rst