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

Handling of local time around DST #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Handling of local time around DST #3

wants to merge 2 commits into from

Conversation

ichernev
Copy link

@ichernev
Copy link
Author

@mj1856 commented (via slack)

WRT ambiguous/invalid, what we're currently doing in moment-timezone, is the same for what Noda Time and Java 8 do, and what Maggie and I have pushed for ECMAScript to define for the Date object. That is as follows:

  1. Ambiguous Local Times: Use the first occurrence. This can be caused by any backward moving transition, but it's important to note that in the case of a DST fall-back transition this will always be the daylight time - not the standard time. For example, in America/Los_Angeles, 2017-11-05T01:30:00 should be interpreted as 2017-11-05T01:30:00-07:00 (PDT).

  2. Invalid Local Times: Skip forward by the amount of the transition gap. In most DST spring-forward transitions, this will be one hour. However, Australia/Lord_Howe has a 30-minute DST transition, and other types of transitions can be arbitrarily long. Therefore, don't hard-code one hour, but lookup the length of the transition gap to determine the next local time. Another way to think about this is to pretend the transition hasn't happened yet, and then normalize to the same UTC time. For example, in America/Los_Angeles, 2017-03-12T02:30:00 should be interpreted as 2017-03-12T02:30:00-08:00, which doesn't exist in local time so it normalizes to 2017-03-12T03:30-07:00 because those both share the same UTC time of 2017-03-12T10:30:00Z. And since there's a one-hour DST gap, you can see the local time of 02:30 resulted in a shift of one-hour to 03:30. Again, the final value will always be in daylight time if the transition was due to DST.

  3. These choices are sensible defaults for the vast majority of applications, especially those involving scheduling. There are few reasons to deviate from the defaults, so one has to wonder how valuable an API is that allows for this behavior to be configurable. Still, they do exist - for example, look at ZoneLocalMappingResolver in NodaTime, which defines the "resolvers" used when transitioning from a LocalDateTime to a ZonedDateTime.

  4. Why do you have Set using different small/big units than add/subtract? In both cases, hours or less should be smaller units that are "UTC correct" while days or more should be bigger units that are "local correct"

  5. For start of day, you say to "pick the future one". It should simply follow the same semantics as above, which would pick the first one. Indeed, it makes no sense to say the start of the day is an hour later than the actual start of the day. Even if the hours between 00:00 and 01:00 are repeated, those are both on the same day. (AFAIK, nobody has ever done something stupid like a one-hour backward transition at 00:30, which would make the "day" ambiguous!)

  6. For end of day, yes - pick the last valid local time on the day. If it's ambiguous, then it would be the second one. This matters only to us because we return 23:59:59.999 as the end of the day, so it's basically the start of the next day, less the smallest precision we offer, using "UTC correct" math. (edited)

  7. BTW, the terminology you used is interesting. What you are saying with "bigger (local correct) units vs. smaller (UTC correct) units", is often explained as "date math vs. time math", or "calendar math vs. timeline math", or "civil time vs. absolute time". It basically all means the same thing though. One moves with date units as if on a calendar, and with time units as if on a clock. Really it's only us programmers that try to blend these concepts together into a single metric with different precision. 🙂

@ichernev
Copy link
Author

  1. Ambiguous Local Times: Use the first occurrence. This can be caused by any backward moving transition, but it's important to note that in the case of a DST fall-back transition this will always be the daylight time - not the standard time. For example, in America/Los_Angeles, 2017-11-05T01:30:00 should be interpreted as 2017-11-05T01:30:00-07:00 (PDT).

By first you mean older? Also isn't daylight happening in the summer, which could be during winter months (in southern hemisphere), but it still moves forward then backwards, never in the other direction? Also when you say DST, can the time move and not be called DST (like dictator wants to align time with X... so no DST but time shift instead).

  1. Invalid Local Times: ...

I got that, but then again -- you imply there are transitions not due to DST, I guess we need to use the same logic as for DST transitions.

  1. These choices are sensible defaults for the vast majority of applications, especially those involving scheduling. There are few reasons to deviate from the defaults, so one has to wonder how valuable an API is that allows for this behavior to be configurable. Still, they do exist - for example, look at ZoneLocalMappingResolver in NodaTime, which defines the "resolvers" used when transitioning from a LocalDateTime to a ZonedDateTime.

For scheduling, isn't it safer to assume end times are the latter option for ambiguous (so the worker is not stripped one hour)? I wasn't aware that other libs were doing more or less the same, given how arbitrary browsers behave.

  1. Why do you have Set using different small/big units than add/subtract? In both cases, hours or less should be smaller units that are "UTC correct" while days or more should be bigger units that are "local correct"

If now is 01:00, there is dst at 02:59->04:00 and you set the hour to 06, then if its UTC accurate it should add 5 hours, which would result in 07:00, but if its local accurate it would result in 06:00 (because it will try to be local accurate, adjust the offset and be happy). With UTC accurate there is only adding/subtracting (otherwise its not clear what to do). I have seen Safari do the UTC accurate hour (you Date.setHours(X), and the result is X+-1). So that is messed up! For adding my logic is -- if you add 1 hour -- that is pretty well defined unit, so the time that results should really be 1 hour away. Versus if you set, you want the final result to be with that hour, and there is no "correctness" to follow, other than preserve all other units if possible.

  1. For start of day, you say to "pick the future one". It should simply follow the same semantics as above, which would pick the first one. Indeed, it makes no sense to say the start of the day is an hour later than the actual start of the day. Even if the hours between 00:00 and 01:00 are repeated, those are both on the same day. (AFAIK, nobody has ever done something stupid like a one-hour backward transition at 00:30, which would make the "day" ambiguous!)

Well, I'm pretty sure there are DST transitions happening on 00:00 of the first of the month. I'm also talking more about a "hole" like 23:30 -> 00:30 (or even 23:59->01:00) If the time 00:00 is valid (i.e -- not invalid, but could be ambiguous) then picking the first one makes sense. Now that I think about it the default logic should be fine, but people keep complaining that moment doesn't handle start/end of around DST very well.

  1. For end of day, yes - pick the last valid local time on the day. If it's ambiguous, then it would be the second one. This matters only to us because we return 23:59:59.999 as the end of the day, so it's basically the start of the next day, less the smallest precision we offer, using "UTC correct" math. (edited)

Ah, so here is a deviation from the rule above (pick the second offset, when ambiguous). I'm just saying we have to have logic to handle choice, then why not make it user-configurable (in a sane way).

  1. BTW ...

Because I treat hours differently, as you saw earlier, and I didn't know the right terminology, I used mine :)

@ichernev
Copy link
Author

ichernev commented May 25, 2017

@icambron commented (via slack)

  1. I adjusted Luxon's lts->ts+offset logic to be more comparable: https://github.com/icambron/luxon/blob/master/src/datetime.js#L54. Perhaps that is helpful.

  2. I don't quite understand the logic in localToOffsets. It makes sense to explore UTC timestamps before and after but you need to check whether those timestamps and their corresponding offsets actually add up to the local ts. I guess that happens in the caller?

  3. I guess in your proposal we'd need to poll for localToOffsets() each time we have a new local ts, see that there are two offset, and then choose between them. I'm not saying I've measured a performance problem there (I've never even tried it), but it seems like a lot of work when the vast majority of the time you're just going to take the obvious offset anyway, since most times aren't ambiguous. I'm wondering if getting exactly consistent ambiguity behavior matters that much. (That's why Luxon has weird ambiguity resolution behavior; it doesn't check for alternatives so if it guesses one side of the ambiguity and that works, it sticks with it and never even finds out it's ambiguous).

  4. I don't love the term "invalid" here because we use invalid to mean other stuff too, like calendar dates that don't exist, like March 38th. Is that a widely used term of art here or can we change it to something like "hole"?

  5. I've been saying something like "higher order, variable length units use calendar math but lower-order, fixed-length units use timestamp math". I like those terms but really I just think we should pick a set of terms and stick with them, so I'm up for anything

@icambron
Copy link
Member

On Matt's point 4, I think one way to clarify Iskren's point is that setters don't use units at all. For example, moment().set('hour', 4) isn't using "hour" as a unit. Instead, it just sets the hour component of the local time. Since setting that fully specifies the local timestamp the same way adding, say, a day does, you can call that "local correct". But there's not really a "UTC correct" alternative.

@ichernev
Copy link
Author

@icambron setting can always be viewed as adding the difference, and that could be UTC correct. That is how the lower units (minute, second, ms) are implemented. Or, you could argue, its not implemented like that, its like a brand new local time, but then -- how do you handle invalids and ambiguities? And you can handle them the same way as a brand new time.

Rethinking most of my document, if there is already a preferred handling for invalid/ambiguities (pick daylight, approx), the doc can be seriously shortened. But if we want to support custom handling of those then at least presenting the possible options is necessary.

@icambron
Copy link
Member

@ichernev Like, if it's 1:00, then setting the hour to 4 could be done as adding 4-1 hours to the current timestamp? That's true, but seems unfathomably strange. In any case, we agree on how it should work.

@ichernev
Copy link
Author

@icambron i agree its strange, but all other units work this way (while its mostly compatible with the pure setting perspective), even in Date. Setting the month basically adds 30-1 daya so from 31 jan setting feb you can end up in march.

@mattjohnsonpint
Copy link

  1. Ambiguous Local Times: Use the first occurrence....

By first you mean older? Also isn't daylight happening in the summer, which could be during winter months (in southern hemisphere), but it still moves forward then backwards, never in the other direction? Also when you say DST, can the time move and not be called DST (like dictator wants to align time with X... so no DST but time shift instead).

Yes, use the older of the two occurrences. DST is always in summer, it's just that the months that comprise "summer" in the Northern hemisphere are different than those in the summer hemisphere. (Don't think of it as winter.)

  1. Invalid Local Times: ...

I got that, but then again -- you imply there are transitions not due to DST, I guess we need to use the same logic as for DST transitions.

Yes, a transition can occur that is not related to DST, such as a change in the standard-time offset. These are usually singular transitions, rather than being paired like DST. An example would be when Venezuela moved from +4:30 to +4:00 in 2016 by advancing their clocks forward 30 minutes.

All transitions should be handled the same, regardless of their cause. They are either forward-moving (creating a gap of invalid local time) or backward-moving (creating an overlap of ambiguous local time), and the amount of the gap/overlap can be anything.

  1. These choices are sensible defaults for the vast majority of applications, especially those involving scheduling...

For scheduling, isn't it safer to assume end times are the latter option for ambiguous (so the worker is not stripped one hour)? I wasn't aware that other libs were doing more or less the same, given how arbitrary browsers behave.

By "scheduling" I mean more like scheduling an automated task that has some recurrence pattern, such as "do this thing daily at 2:00 am". It makes no sense to skip over the first occurrence and wait for the second one.

WRT scheduling of people taking shifts, @maggiepint and I discussed this, since we both worked in this space previously. Typically, a worker is either scheduled to work through the DST period in entirety, thus earning an extra hour pay and creating no ambiguity, or is explicitly scheduled to end before the DST period begins. In the rare case that they would be scheduled for a portion of the extra DST period (ex: ending their shift at 1:30 am), if they were required to work until the second occurrence, then the scheduling system should deliberately call that out. It's not something we would want to choose as a default.

  1. Why do you have Set using different small/big units than add/subtract? ...

If now is 01:00, there is dst at 02:59->04:00 and you set the hour to 06, then if its UTC accurate it should add 5 hours, which would result in 07:00, but if its local accurate it would result in 06:00 (because it will try to be local accurate, adjust the offset and be happy). With UTC accurate there is only adding/subtracting (otherwise its not clear what to do). I have seen Safari do the UTC accurate hour (you Date.setHours(X), and the result is X+-1). So that is messed up! For adding my logic is -- if you add 1 hour -- that is pretty well defined unit, so the time that results should really be 1 hour away. Versus if you set, you want the final result to be with that hour, and there is no "correctness" to follow, other than preserve all other units if possible.

I never think of setters as adders. It's "assign a value to this field", then "adjust to a valid instant if invalid or ambiguous". If a moment is in UTC mode, then there's nothing to do but set. In local mode, we're currently relying on the Date object's behavior for moment, but we are going through the ambiguous/invalid logic in moment-timezone.

  1. For start of day, you say to "pick the future one". It should simply follow the same semantics as above, which would pick the first one...

Well, I'm pretty sure there are DST transitions happening on 00:00 of the first of the month. I'm also talking more about a "hole" like 23:30 -> 00:30 (or even 23:59->01:00) If the time 00:00 is valid (i.e -- not invalid, but could be ambiguous) then picking the first one makes sense. Now that I think about it the default logic should be fine, but people keep complaining that moment doesn't handle start/end of around DST very well.

A transition at 00:00 doesn't create an ambiguous date. It goes from 23:59:59.999 back to 23:00:00.000. The hole you describe is technically possible, but impractical and non-existent in real life.

  1. For end of day, yes - pick the last valid local time on the day. If it's ambiguous, then it would be the second one...

Ah, so here is a deviation from the rule above (pick the second offset, when ambiguous). I'm just saying we have to have logic to handle choice, then why not make it user-configurable (in a sane way).

Yep. End-of-time is special cased.

@mattjohnsonpint
Copy link

Honestly, I don't understand why we are re-hashing how to handle these things. Really we just need to clean up the interface between moment and moment-timezone.

@ichernev
Copy link
Author

ichernev commented Jun 3, 2017

@mj1856 well, cleaning it up involves the part where we write in moment how to "interpret" the moment-timezone (or other plugin) data, which will be in the form of (at least) UTC-timestamp -> utc-offset. So I got it, that we should use the default behavior when creating/setting and the described behavior during add/subtract and startof/endof.

But there are still questions about what other functions to provide (like previous/next offset change), so moment doesn't have to do binary search to figure out how big the time-shift is (as you said -- it could be any amount, not just 1h).

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.

3 participants