Details
-
Type:
RFC
-
Status: Adopted
-
Resolution: Unresolved
-
Component/s: DM
-
Labels:None
Description
I propose that we drop support for UTC in C++ code and only use TAI. To convert to other time systems we should use Astropy in Python.
The simplest proposal is to remove UTC and TT support in daf::base::DateTime.
A more invasive and forward-looking proposal is to adopt, Pim Schellart [X]'s an astropy-friendly extension to std::chrono in place of DateTime. I lean towards the simpler solution for this RFC, but am happy to hear other opinions.
I searched our code base and found no usage of TT and some usage of UTC in ctrl_events, ctrl_provenance, pex_logging, pipe_drivers and some obs_ packages (especially for ingest). Some of this surprises me, as I thought we had standardized on TAI.
Attachments
Activity
If ctrl_provenance needs a time, it should use astropy as well. I just got shot down for saying something is unused, but I'm pretty sure ctrl_events can be considered such and does not need to be updated.
I am a tiny bit concerned that astropy will get conversions wrong if we do not keep it up to date, although with warnings (that could also be false positives). Hopefully they will provide for an external source to eliminate this.
I have no objections to the rest.
Russell Owen it sounds like we have an agreed plan and can adopt this RFC. Please update the RFC text with the final proposal.
Adopted as follows:
1) Remove support for all time systems except TAI from lsst::daf::base::DateTime.
2) Modify DateTime.toPython to return an astropy.time.Time.
3) Add class or static method DateTime.fromPython to construct a DateTime from an astropy.time.Time in Python
If packages need conversion between time systems they can use astropy.time.
I am not working for DM any longer. If this still needs to be implemented, somebody else should do it.
For logs I think it makes the most sense to log in whatever time system the computer is using. I also don't think the logging system should be using DateTime which would decouple the issue of logged time stamps from what DateTime does and allows us to eliminate UTC support from DateTime.
For purposes of closing this RFC I propose the following:
1) Remove support for all time systems except TAI from DateTime.
UTC is used in ctrl_events and ctrl_provenance and some obs_ packages. I propose that the ctrl_ packages use TAI. The usage in obs packages can be replaced by Astropy.time.Time.
2) Modify DateTime.toPython to return an Astropy.time.Time
...as per Jim Bosch's excellent suggestion. A global search turned up only 4 uses in 4 packages: daf_base, daf_butler, obs_subaru and pipe_drivers. So I think this will be easy.
3) Add class or static method DateTime.fromPython to construct a DateTime from an Astropy.time.Time.
This is simpler than Jim Bosch's suggestion, but it seems a pragmatic and easy first step. I would like to defer automatic conversion between DateTime and Astropy.time.Time (in either direction) to another RFC bcause I fear that it will be too far easy to forget to make new pybind11 wrapper take and return Astropy.time.Time instead of DateTime, leading to some code that deals with DateTime and some code that deals with Astropy.time.Time. Unless there is some clever way to make the obvious pybind11 wrapper manage the transition automatically, I think this is asking for trouble.
Thus I think it most important that we can easily manually convert between DateTime and Astropy.time.Time and less important that we do it automatically.