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

Clarify implied timezone in stop_times.txt #378

Merged
merged 4 commits into from Aug 1, 2023

Conversation

tzujenchanmbd
Copy link
Collaborator

@tzujenchanmbd tzujenchanmbd commented May 17, 2023

The purpose of this PR is:

  • to clarify the usage of time zone for stop_times.arrival_time and stop_times.departure_time
  • to avoid producers providing time based on incorrect time zone
  • to prevent potential confusion that may arise after introduction of time-variable fares

Current definition on stop_times.arrival_time:

Arrival time at the stop (defined by stop_times.stop_id) for a specific trip (defined by stop_times.trip_id). If there are not separate times for arrival and departure at a stop, arrival_time and departure_time should be the same. For times occurring after midnight on the service day, enter the time as a value greater than 24:00:00 in HH:MM:SS local time for the day on which the trip schedule begins.

Problem (quoted from Elizabeth in #322):
The above definition, with use of the word "local" implies that stop_times.txt should use the time zone of the local stop. However, the stop.stop_timezone definition clearly states that if used, that stop_times should refer to agency.agency_timezone.

This problem caused Amtrak to provide incorrect data in 2022.

Proposed solution:
Clarify definition of time, stop_times.arrival_time, and stop_times.departure_time

Fixes issue#322 (see issue#322 for previous discussion)

Clarify implied timezone for time values at stop_times.arrival_time & stop_times.departure_time
Removed "local time" from Field Type: Time
@google-cla
Copy link

google-cla bot commented May 17, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@emmambd emmambd linked an issue May 17, 2023 that may be closed by this pull request
@@ -76,7 +76,7 @@ Presence conditions applicable to fields and files:
- **Float** - A floating point number.
- **Integer** - An integer.
- **Phone number** - A phone number.
- **Time** - Time in the HH:MM:SS format (H:MM:SS is also accepted). The time is measured from "noon minus 12h" of the service day (effectively midnight except for days on which daylight savings time changes occur). For times occurring after midnight, enter the time as a value greater than 24:00:00 in HH:MM:SS local time for the day on which the trip schedule begins. <br> *Example: `14:30:00` for 2:30PM or `25:35:00` for 1:35AM on the next day.*
- **Time** - Time in the HH:MM:SS format (H:MM:SS is also accepted). The time is measured from "noon minus 12h" of the service day (effectively midnight except for days on which daylight savings time changes occur). For times occurring after midnight, enter the time as a value greater than 24:00:00 in HH:MM:SS for the day on which the trip schedule begins. <br> *Example: `14:30:00` for 2:30PM or `25:35:00` for 1:35AM on the next day.*
Copy link

@derhuerst derhuerst May 22, 2023

Choose a reason for hiding this comment

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

Because we're already changing this definition, I propose another related change:
I'd argue that the "for the day on which the trip schedule begins" part confuses more than it helps. I think we should either a) state (again here as before) that the time is relative to the service day, or b) don't add any alternative phrase as a GTFS "Time" is always relative to the service day.
Same below in the {arrival,departure}_time definitions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@derhuerst what about the following definition?

Time - Time in the HH:MM:SS format (H:MM:SS is also accepted). The time is measured from "noon minus 12h" of the service day (effectively midnight except for days on which daylight savings time changes occur). For times occurring after midnight on the service day, enter the time as a value greater than 24:00:00 in HH:MM:SS. [remove "for the day on which the trip schedule begins"]

Choose a reason for hiding this comment

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

Sounds good.

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jun 15, 2023
@leonardehrenfried
Copy link
Contributor

I think this is a good change and would welcome further discussion and an eventual merge.

@tzujenchanmbd
Copy link
Collaborator Author

tzujenchanmbd commented Jul 24, 2023

It looks like the proposal does not raise other concerns. I’m opening a vote on this PR. This vote is for modifying description in Timefield type, stop_times.arrival_time, and stop_times.departure_time, as outlined in the first comment.

Please vote with a +1 (for) or -1 (against) in the comments. Voting ends on 2023-07-31 at 23:59:59 UTC.

@tzujenchanmbd tzujenchanmbd added the Status: Voting Pull Requests where the advocate has called for a vote as described in the changes.md label Jul 24, 2023
@npaun
Copy link
Contributor

npaun commented Jul 24, 2023

+1 Transit

@leonardehrenfried
Copy link
Contributor

+1 OpenTripPlanner

@skinkie
Copy link
Contributor

skinkie commented Jul 25, 2023

I would want to make it even more explicit by adding ", not the stop.timezone".

Add "not stops.stop_timezone" for both departure_time and arrival_time
@tzujenchanmbd
Copy link
Collaborator Author

@skinkie thanks for the suggestion!
Added ", not stops.stop_timezone" for both arrival_time and departure_time. - 9d4e426

@e-lo
Copy link

e-lo commented Jul 25, 2023

+1

@doconnoronca
Copy link

+1 TransSee

@westontrillium
Copy link

+1 Trillium

@albanpeignier
Copy link

+1 enRoute

@isabelle-dr isabelle-dr mentioned this pull request Jul 27, 2023
4 tasks
@tzujenchanmbd tzujenchanmbd removed the Status: Voting Pull Requests where the advocate has called for a vote as described in the changes.md label Aug 1, 2023
@tzujenchanmbd
Copy link
Collaborator Author

The vote passed on 2023-07-31 at 23:59:59 UTC.

6 votes in favour and no votes against.

The votes came from:
Transit (@npaun)
OpenTripPlanner (@leonardehrenfried)
Elizabeth Sall (@e-lo)
TransSee (@doconnoronca)
Trillium (@westontrillium)
enRoute (@albanpeignier)

Thanks to everyone who contributed and voted! (Special thanks to @e-lo for bringing up the issue in 2022!)
We are in the process of updating the GTFS Schedule Reference page on gtfs.org.

@tzujenchanmbd tzujenchanmbd merged commit 8c57342 into google:master Aug 1, 2023
2 checks passed
gcamp pushed a commit to TransitApp/transit that referenced this pull request Sep 25, 2023
* arri/dept time updates

Clarify implied timezone for time values at stop_times.arrival_time & stop_times.departure_time

* Removed "local time"

Removed "local time" from Field Type: Time

* Remove "for the day on which the trip schedule begins"

comment from derhuerst - google#378 (comment)

* Add "not stops.stop_timezone"

Add "not stops.stop_timezone" for both departure_time and arrival_time
@isabelle-dr isabelle-dr added the Change: Clarification Revisions of the current specification to improve understanding. label Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Change: Clarification Revisions of the current specification to improve understanding. GTFS Schedule Issues and Pull Requests that focus on GTFS Schedule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify implied timezone for time values in stop_times.txt
10 participants