Skip to content
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-42227: Switch from datetime to astropy.time in MiddlewareInterface #116

Merged
merged 7 commits into from
Jan 26, 2024

Conversation

kfindeisen
Copy link
Member

@kfindeisen kfindeisen commented Jan 25, 2024

This PR removes the use of datetime in MiddlewareInterface in favor of astropy.time, though some unit tests still use datetime as the oracle. It also fixes a bug in our tests that was exposed by lsst/dax_apdb#41.

This change makes it possible for the patches to refer to individual
test cases' environments, in particular their temporary directories.
Some execution paths construct a real ApdbSql object, and the object
may attempt to contact the database it represents. Since neither our
development and build environments don't have a local PostgreSQL
server, use a temporary sqlite DB instead.
astropy.time lets us handle TAI natively, and there's no risk of
accidentally giving an unaware datetime. This will also give us more
precision when Visit.startTime arrives.
The default is not used by any production code, and both raises the
risk of inconsistent collection handling and confuses the program's
decision flow.
This prevents the output run code from depending on which time library
we use to calculate day_obs; it only needs to know its value.
For consistency with the observing day_obs, the value is defined based
on the TAI-12 date of processing start, not the UTC-12 date.
@hsinfang
Copy link
Collaborator

A moment... Still checking if it makes sense to update
https://github.com/lsst-dm/prompt_processing/blob/main/python/tester/upload.py#L293

@kfindeisen
Copy link
Member Author

To be fully self-consistent, we probably should (if I understand things correctly, the translation between YMD and seconds since epoch is affected by the UTC-TAI offseet). Though it's worth noting that the real T&S code uses a homebrew system on top of datetime instead of astropy.time.

@hsinfang
Copy link
Collaborator

hsinfang commented Jan 26, 2024

I confirmed that the DATE-BEG metadata from json or fitsheader is TAI (by checking numbers of one LATISS image and here because I don't know where is the offcial header document).
private_sndStamp is in TAI too.

So.. we (probably I) weren't careful to handle the timestamp as TAI but the the numbers should be fine. And these numbers aren't really used yet.

The private_sndStamp field is populated by converting an ISO 8601
string to a Unix epoch. To avoid any complications in the future, we
should do the conversion in a TAI-aware way.
@kfindeisen kfindeisen merged commit 8ab3583 into main Jan 26, 2024
5 checks passed
@kfindeisen kfindeisen deleted the tickets/DM-42227 branch January 26, 2024 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants