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-23171: Add exposure_group calculation and make visit_id use it #161
Conversation
Exposure group now read from GROUPID header if defined, else uses the exposure_id as a string. Visit has to be changed to match since things with the same exposure_group must now have matching visit id. If exposure_group is a number use it, else derive it from the group ID.
d59f3c8
to
9254cc4
Compare
This is ObservationInfo.exposure_group
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 good. Minor comments.
tdelta = iso - TZERO_DATETIME | ||
epoch = int(tdelta.total_seconds()) | ||
|
||
# Form the integer from EPOCH + 3 DIGIT FRAC + 0-pad N |
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.
You don't mean addition here. It's an append. I agree it's clear from the next line.
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.
Yes. Good point.
TZERO = Time("2015-01-01T00:00", format="isot", scale="utc") | ||
TZERO_DATETIME = TZERO.to_datetime() | ||
|
||
# Regex to use for parsing a GROUPID string |
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.
Maybe add an example string this is meant to match?
visit_id = int(digest, base=16) | ||
|
||
# To help with hash collision, append the string length | ||
visit_id = int(f"{visit_id}{len(exposure_group):02d}") |
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.
Why truncate to two digits?
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.
The length comes from a FITS header value and unless we are going crazy with CONTINUE header cards 72 is the longest group ID.
Exposure group now read from GROUPID header if defined,
else uses the exposure_id as a string. Visit has to be
changed to match since things with the same exposure_group
must now have matching visit id.
If exposure_group is a number use it, else derive it
from the group ID.
Depends on lsst/astro_metadata_translator#35