Repository navigation
Update event_type enum values to snake_case - #247
Conversation
chrisyamas
added
📄 spec
|
Documentation available at: |
chrisyamas
added
the
📙 docs
|
This may end up being a separate issue, but I notice that we have |
|
@botanize proposal in Contributors meeting 8/13: convert this PR to handle all enums so that snake case is adopted uniformly in the 2.0 release |
lauriemerrell
left a comment
lauriemerrell
left a comment
There was a problem hiding this comment.
putting a request changes review on to indicate the need to update this to handle ALL enums
|
Here are the other enums in the spec right now.
|
|
In GTFS schedule I though those enums were numeric so the styling shouldn't matter? (Overall, I accept this change and don't have a strong concern about how the GTFS decision falls, but my vote would be for uniformity in TIDES) |
|
on GTFS route_type, seconding laurie's comment to refer to the ones here: https://gtfs.org/documentation/schedule/reference/#routestxt on NTD mode, we could use the two-letter short codes (MB for bus), etc. Trying to find link, but early pages of the NTD reporting guide typically include this We may need to document as-of dates or version references for these sources in the TIDES spec, since they do change |
|
I'm in favor of using consistent formatting for TIDES defined enums, and adopting enums values as if from other standards. |
|
I wanted to ask @botanize :
so you'd be okay with all snake_case, but okay using NTD (short string initials ala 'MB') and GTFS (numeric) identifiers also wanted to offer @chrisyamas -- i know there's a lot going on, if you want me to take some of the conversation here and fold this into the PR i'd be happy to help next week. i think i missed in the call what form we wanted the crosswalk to take here, but maybe you can fill me in |
|
The consensus from the Sept 10 Contributors Group meeting was that TIDES-defined enum values should convert to snake_case, and values inherited from external vocabularies (including The branch update implementing this scope is now up. Per the change management policy, the revised proposal recirculates for at least 72 hours with previous reviewers notified. @botanize a new look at this when you have a moment would be great since your approval predates the expanded scope, and @lauriemerrell with the scope question settled, your earlier changes-requested marker can clear whenever suits. Thank you all! |
| "All doors opened", | ||
| "Other configuration" | ||
| "doors_did_not_open", | ||
| "front_door_opened_and_back_doors_remain_closed", |
There was a problem hiding this comment.
| "front_door_opened_and_back_doors_remain_closed", | |
| "front_door_opened_and_back_doors_remained_closed", |
| "type": "string", | ||
| "title": "ID referencing operators.operator_id", | ||
| "description": "Identifies the vehicle’s operator." | ||
| "description": "Identifies the vehicle\u2019s operator." |
There was a problem hiding this comment.
nit: replace with ascii apostrophe throughout? also made this on other PR, saw here too
| "Monorail/Automated Guideway", | ||
| "Other", | ||
| "Público", | ||
| "P\u00fablico", |
There was a problem hiding this comment.
question: i saw the comment in the changelog about preserving source references for NTD, but couldn't we make these use the NTD short codes instead? (Bus -> MB, Commuter Rail -> CR) I think that would derisk weird formatting issues (as we're trying to do in this PR) and still maintain the convention from NTD. Don't know how many TIDES publico reporters we'll get, but "PB" seems easier than having non-ascii characters in the spec and TIDES data, for instance.
A source reference for NTD modes could be found here: https://www.transit.dot.gov/sites/fta.dot.gov/files/2026-06/2026-NTD-Full-Policy-Manual_0.pdf (see footer page 29/ pdf page 47)
I think one challenge is that these modes do evolve from year (BRT got its own mode at one point, etc.). I think for TIDES reporters we could probably safely adopt the 2026 terms, then decide later how to handle evolutions in what NTD defines.
We could also defer this to another ticket to get the 2.0 update out--and i realize this is not a snake_case matter--but it seems fairly straightforward to me and the enum PR seems like it could be the right place.
There was a problem hiding this comment.
Agreed. Using the NTD two characters codes is what I thought we were moving towards here. (But I also got a little confused in the conversation). I also agree that this will continue to need some adjustment over time, especially beyond the scope of FTA/NTD.
|
I'd like to get to consensus on the last two fields so we can resolve this PR. For The There are several different options for how to proceed with Option 1: Leave the field as is. Come back to it at a later date. I would vote for Option 1 for inclusion in v2.0-alpha.1. Please reply with your thoghts or suggestions. |
|
thanks for following up, christopher! Looks fine to me. Maybe we add somewhere in docs that we're referring to the 2026 NTD mode vintage, since that could change later, but not a big deal. i'm okay to move on, approved |
|
I would prefer to see the GTFS numeric codes used for route type in the pre-release |
botanize
left a comment
botanize
left a comment
There was a problem hiding this comment.
I recommend simplifying door_status and implementing all enum changes at this time.
| "doors_did_not_open", | ||
| "front_door_opened_and_back_doors_remained_closed", | ||
| "back_doors_opened_and_front_door_remained_closed", | ||
| "all_doors_opened", | ||
| "other_configuration" |
There was a problem hiding this comment.
I think we could simplify these considerably, and might want to change the name of the field to door_action:
| "doors_did_not_open", | |
| "front_door_opened_and_back_doors_remained_closed", | |
| "back_doors_opened_and_front_door_remained_closed", | |
| "all_doors_opened", | |
| "other_configuration" | |
| "none", | |
| "front_only", | |
| "back_only", | |
| "all", | |
| "other" |
There was a problem hiding this comment.
I like these new values and the suggestion to change the field name to door_action
| "type": "string", | ||
| "description": "Indicates the type of transportation used on a route. References GTFS routes.route_type including Google's [Extended Route Types](https://developers.google.com/transit/gtfs/reference/extended-route-types) GTFS extension", | ||
| "constraints": { | ||
| "enum": [ |
There was a problem hiding this comment.
Strongly recommend we change these to the numeric values used by GTFS now and not push this off.
There was a problem hiding this comment.
I agree with @botanize comments but don't need to independently verify the outcome, not sure if I should mark that as "approved" on the PR or leave myself at commented.
ETA: Below should say "simplicity" 😛
Defer to Joey's requested changes status for simplicy
Co-authored-by: John Levin <[email protected]>
Co-authored-by: Joey Reid <[email protected]>
|
Thanks @botanize for the review, and @lauriemerrell for weighing in too! I think it makes sense to take both of these now while these enums are already changing, so implementers only have to update once.
I'll push this shortly and follow up here when the new review window starts. And @wtimmerman-fitp, good call on the 2026 NTD mode vintage, that's going into the migration guide (#274) and the alpha.1 CHANGELOG entry. |
70d5bdb to
4f44af3
Compare
|
Data Validation Report
|
|
Just wanted to give a heads up to @botanize @lauriemerrell @jlstpaul @wtimmerman-fitp @CTRAN-Sutinen that this proposal has been revised again in response to Joey's review, so it's re-circulating per the change management policy (at least 72 hours, previous reviewers notified). With this round of changes, If nothing here gives you pause, it will go ahead and proceed through review in accordance with the CMP, and fresh reviews are welcome either way. @botanize a new look when you have a moment would be great, and @lauriemerrell if an approval on the new head works for you, that would definitely help, though no pressure if you'd rather stay at commented. Thank you all! |
|
A recap on where this PR stands before we proceed with merging. The revision that followed Joey's review re-circulated for more than 72 hours with no objections, per the change management policy, and it has fresh approvals on the current head from Joey and Wylie, with the CHANGELOG entry in place. So I'm going ahead and merging it into |
6 participants
Summary
This is a normative, breaking change converting enum values from 'Sentence case' to snake_case across all TIDES-defined vocabularies, for code compatibility. Scope follows the community's decisions, including @botanize's question in #235, the agreement at the Aug 13 Contributors Group meeting to finish the job on this in v2.0, and the consensus reached at the Sept 10 Contributors Group meeting determining that enum sets inherited from external vocabularies should keep their source form. Following the review discussion here,
trips_performed.ntd_mode(NTD) now uses the official NTD two-character mode codes (MB,CR,PB, and so on), withDemand TaxiandOtherremoved since they have no NTD code, andtrips_performed.route_type(GTFS) now uses the numeric route type codes GTFS itself uses, including Google's Extended Route Types. This resolves #235.Changes
Changes include:
device_type;fare_actionandfare_media; all threeschedule_relationships;train_car_type;trip_typein both tables;current_status;gps_quality), withdevice_events.event_typecoming in through Rename passenger_events to device_events with expanded scope #251operator_button_pressntd_modeswitched to the 19 NTD two-character mode codes, and the field is left empty when a trip has no NTD modestop_visits.door_statusrenameddoor_action, with shorter values (none,front_only,back_only,all,other) and a short meaning for each in the field description (Joey's suggestion, with John agreeing)trips_performed.route_typebecomes an integer field using the GTFS numeric route type codes, including Google's Extended Route Types (91 codes, covering every value the old list allowed plus 403 and 405, whose names duplicated other entries)fare_action's per-value glossary is updated to matchValue mapping tables, including a name-to-code table for
route_type, will be included in the migration guide (#274).direction_idis already numeric and is untouched in these changes.Review checklist
Per change management policy, the following must be met before feature branch changes can merge to
developbranch:Community review status (updated October 14, 2026)
How community review works: Per the TIDES Change Management Policy, a proposal moves forward once at least three TIDES Contributors outside the originating working group publicly comment with a score: Accepted, Accepted with minor changes, or Substantially revised. A few sentences with your read of the proposal is a complete review.
Originating working group (Fall 2025 Issues Working Group, December 1 and 10, 2025 sessions): Christopher Yamas, Gabriel Sánchez Martínez, Ian Thistle, Jay Gordon, John Levin, Joey Reid, Spenser Sutinen. Their work is reflected in the proposal itself; community review comes from Contributors beyond this group.
Review so far: @lauriemerrell accepted this change and the Sept 10 Contributors Group meeting settled the scope (TIDES-defined values convert; external vocabularies keep their source form). The full conversion landed September 14 and is re-circulating per the change management policy, with previous reviewers notified. Fresh reviews welcome. A follow-up revision on Oct 7 moved
ntd_modeto the NTD two-character codes and accepted John'sremained_closedsuggestion. A further revision on Oct 14, following Joey's review, renameddoor_statustodoor_actionwith shorter values and movedroute_typeto the GTFS numeric codes, and the branch was rebased ontodevelopafter #249, #248, #250, #279, and #251 merged, so the proposal is re-circulating again.The revised proposal is re-circulating for at least 72 hours per the change management policy; if nothing gives you pause it proceeds, and comments are always welcome.
The full v2.0 map lives on #271.