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-7587: Time scale fixes #12

Merged
merged 2 commits into from Sep 13, 2016
Merged

DM-7587: Time scale fixes #12

merged 2 commits into from Sep 13, 2016

Conversation

r-owen
Copy link
Contributor

@r-owen r-owen commented Sep 13, 2016

No description provided.

@timj timj changed the title Tickets/dm 7587 DM-7587: Update time scale fixes Sep 13, 2016
@timj timj changed the title DM-7587: Update time scale fixes DM-7587: Time scale fixes Sep 13, 2016
@@ -63,15 +63,15 @@ class DateTime {
explicit DateTime(double date, DateSystem system = MJD, Timescale scale = TAI);
DateTime(int year, int month, int day, int hr, int min, int sec,
Timescale scale = TAI);
explicit DateTime(std::string const& iso8601);
explicit DateTime(std::string const& iso8601, Timescale scale);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No default? Does the name iso8601 still make sense for all possible values of Timescale?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://dotat.at/tmp/ISO_8601-2004_E.pdf does mention TAI is fine in this format.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only choices DateTime supports are TAI, TT and UTC. TT is just an offset from TAI. So I think it makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timj While the document mentions TAI as a possible timescale, it's a little wishy-washy about how it should be represented. Note that 4.2.1 mentions non-UTC timescales but no particular representation, and none of the subsequent subsections that do give representations seem appropriate for a TAI time that is not a local time nor a "UTC of day".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It's a bit loose. http://docs.astropy.org/en/v1.2.1/time/index.html has some examples and I remember getting very confused when plugging astropy.time in as a backend for DateTime.

Add a scale argument to DateTime's ISO string constructor
and toString method. It has no default, so existing code
must be retrofitted.
The constant TT_MINUS_TAI_NSECS was being approximated by a double
instead of using a long long, resulting small errors.
@r-owen r-owen merged commit 5d86e5d into master Sep 13, 2016
@ktlim ktlim deleted the tickets/DM-7587 branch August 25, 2018 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants