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

Use local time fields #76

Closed
wants to merge 1 commit into from
Closed

Use local time fields #76

wants to merge 1 commit into from

Conversation

dfaulken
Copy link
Contributor

@dfaulken dfaulken commented Mar 3, 2018

My understanding of the new local time fields is that they will remain consistent during a service day, even if that service day includes a time change.

Therefore, we can switch to using the local time fields, and discard our logic which we had constructed to deal with the feed error. Closes #9.

Correct me if I'm wrong.

My understanding of the new local time fields is that they will remain consistent during a service day, even if that service day includes a time change.

Therefore, we can switch to using the local time fields, and discard our logic which we had constructed to deal with the feed error. Closes #9.

Correct me if I'm wrong.
@dfaulken dfaulken self-assigned this Mar 3, 2018
@dfaulken dfaulken requested review from werebus, sherson and Anbranin March 3, 2018 13:03
@sherson
Copy link
Member

sherson commented Mar 3, 2018

@dfaulken commented:

My understanding of the new local time fields is that they will remain consistent during a service day, even if that service day includes a time change.

That is also my understanding. In the email from Avail last July:

The new fields will follow the service days’ time as our departures are built at the beginning of the day so the time component being shown will follow service date.

Based on that, the LocalTime fields will not "spring forward" from 1:59:59 to 3:00:00 (or "fall back" from 1:59:59 to 1:00:00 in November).

It's worth noting that at 1:55, a bus with an EDT of 2:05 (service date time) would technically be leaving in 10 min at 3:05 am in the spring (and in 10 min at 1:05 am in the fall), but I think it's fine to display the time as 2:05 am. Whichever time we display could end up confusing people, but hopefully the 10 min part of the departure board will make it clear.

Side note: I just realized that this won't be an issue until November anyway due service ending early for Spring Break (the March DST switch didn't happen during Spring Break in 2014 and 2015, but it did in 2016 and 2017, and will in 2018 and 2019).

@dfaulken
Copy link
Contributor Author

dfaulken commented Mar 3, 2018

but hopefully the 10 min part of the departure board will make it clear

So then this PR goes too far. When the current local time switches for instance from 2:00 am to 1:00 am, then a bus scheduled for 2:05 will show an absolute time of 2:05 am, but a relative time of "1 hr, 5 min", despite the fact that it arrives in 5 minutes.

So we still need to maintain our offset logic, but on the other end of the time display. We need to remember what the offset was at the start of the day for the purposes of showing relative times, whereas previously we did it for the purposes of showing absolute times.

Correct?

@sherson
Copy link
Member

sherson commented Mar 3, 2018

I don't have a strong opinion about which time-of-day we show (if anyone else does, please chime in), but the minutes away should be accurate.

@sherson sherson mentioned this pull request Mar 3, 2018
@sherson
Copy link
Member

sherson commented Mar 3, 2018

Thinking about it a bit more, it might make more sense to show the times as 2:05 in my example (simply because that would correspond to the scheduled time).

@dfaulken
Copy link
Contributor Author

dfaulken commented Mar 4, 2018

Okay. @werebus, does my analysis of what needs to be done here sound correct to you?

@werebus
Copy link
Member

werebus commented Mar 5, 2018

Yes, we still need the offset. Although, alternatively, it might be better to know what UTC offset to use for the service day instead. Moment still knows something about DST even with "local" times:

moment('2018-03-11T01:30:00').format('h:mm a')
//=> "1:30 am"
moment('2018-03-11T02:30:00').format('h:mm a')
//=> "3:30 am"
moment('2018-03-11T03:30:00').format('h:mm a')
//=> "3:30 am"

That is, when we parse "Local" times for math purposes from Avail they should be LocalTimeField + -0n00 where "n" is 4 or 5 depending on the start of the service day:

moment('2018-03-11T01:30:00-0500').format('h:mm a')
//=> "1:30 am"
moment('2018-03-11T02:30:00-0500').format('h:mm a')
//=> "3:30 am"
moment('2018-03-11T03:30:00-0500').format('h:mm a')
//=> "4:30 am"

But, as is evident from my second example, we should (I guess) display them unmodified. Those times are on the printed timetables as "1.30", "2:30", and "3:30" respectively.

@werebus
Copy link
Member

werebus commented Jun 4, 2021

This PR is from more than 3 years ago and isn't correct yet. I think I'm going to close it. If someone wants to work on #9 still, it would probably be better to start from scratch.

@werebus werebus closed this Jun 4, 2021
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.

DST Switchover Behavior
3 participants