New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DM-42636: add group and day_obs first-class dimensions #953
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #953 +/- ##
==========================================
- Coverage 88.53% 88.52% -0.01%
==========================================
Files 313 313
Lines 40036 40182 +146
Branches 8371 8407 +36
==========================================
+ Hits 35444 35570 +126
- Misses 3379 3392 +13
- Partials 1213 1220 +7 ☔ View full report in Codecov by Sentry. |
0449ec0
to
bbac61a
Compare
b1ec31a
to
3458a15
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great; I especially appreciate the effort to reconstruct the dimension universe changelog.
(I can't hit approve because I created the PR.)
e415293
to
26df6f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New changes look good! (Approved again in spirit.)
Previously, the only way to reconstruct the history was to run git blame on dimensions.yaml. lsst/daf_butler#953 will add human-curated documentation to the Pipelines docs.
Previously, the only way to reconstruct the history was to run git blame on dimensions.yaml. lsst/daf_butler#953 will add human-curated documentation to the Pipelines docs.
Previously, the only way to reconstruct the history was to run git blame on dimensions.yaml. lsst/daf_butler#953 will add human-curated documentation to the Pipelines docs.
The offset was meant to be "offset * (n + 1)" not an offset of n minutes plus one day.
This file was so old it predated the addition of day_obs to the exposure and visit dimensions, and that was back before we even had a dimension universe version. While it's nice to have the YAML import code migrate simple things, it has no idea where an instrument is and hence can't handle this one. Procedure was: - make an empty SQLite repo - import the old file - query for visit dimensions (there are no exposures in this file) - make new records with day_obs set, by looking at visit.timespan.begin and doing some timezone stuff (HSC is in Hawaii, which I just hard-coded here) - re-insert the new visit recods with replace=True - export all of the dimension elements in butler.dimensions.elements with element.has_own_table=True.
This updates the YAML file to have day_obs as a dimension rather than rely on the import-time migration, which warns.
This ensures we do not try to load exposure before we've loaded day_obs.
We want SpecificSerializedDimensionRecordDayObs and not SpecificSerializedDimensionRecordDay_obs.
Only back to version 1.
This required that code is added to derive day_obs from the datetime_begin field (something which should have been added long ago when universe version 1 came out).
Instead always try to get the offset from the attached instrument class's metadata translator.
26df6f2
to
58e02cb
Compare
Depends on lsst/astro_metadata_translator#71.
Checklist
doc/changes