-
Notifications
You must be signed in to change notification settings - Fork 17
Best Practice and spec alignment #45
Comments
☑️ I think that would be very helpful.
❓ So this would be a |
Currently We would be adding, into the spec, the recommendation to provide @isabelle-dr I'm not sure how recommendations are normally treated in the validator? |
Combining the spec and best practice would be something like... ~
|
RE |
|
@scmcca thank you for working on this 🙏
The recommendations (i.e. mention of should) trigger a notice with a severity WARNING. The Conditionally Required fields are treated the same way we treat the Required fields and the mentions of must: the validator emits a notice with an ERROR severity if the requirement is not met. What you're offering to do with
That being said, that second ERROR isn't currently represented in the validator because we weren't sure how to write it without potential false positives I think? @lionel-nj |
I think there is a risk of losing information if we make a one-time declaration about fields.
Also, it would be unclear what to do with the validator: should we then emit a WARNING for all optional fields that aren't included? If so, the report would be noisy and the more important warnings risk being lost.
Making a one-time declaration about files: that's a good idea IMHO. |
As GTFS Best Practices (BP) are currently in the process of being merged to the specification, MobilityData is migrating outstanding issues and PRs from this repository to google/transit. Thus, this issue will be closed and further discussion regarding this BP should take place in google/transit. Please refer to Issue #421 for a more detailed explanation of the migration process and the proposed next steps. With this, we’re hoping to bring more visibility to outstanding BP issues and to restart the discussion around them, so that any improvement that the community finds valuable could be carried forward. |
Best Practice and spec alignment
Along with my review provided in #44, here is a list of conflicts/opportunities to resolve for consistency/quality between the Best Practices and the spec.
All Files
agency.txt
agency.agency_id
:agency_id
: “Should be included, even if there is only one agency in the feed.” --> Move this into spec? (likely won't be able to require since it is optional in the spec for single-agency datasets)stops.txt
stop_lat
&stop_lon
: Different (similar) recommendations exist for positional accuracy in the BP and the spec. We should streamline to one. IMHO the statement in the BP is more descriptive and should be merged into the spec.Spec recommendation currently reads:
Best Practice recommendation currently reads:
routes.txt
agency_id
: "must be included" is false according to the spec's "Conditionally Required" statement. Needs to be rationalized with BP for includingagency_id
in single-agency datasets.route_id
: "All trips on a given named route should reference the same route_id." should be a recommendation in trips.txt?route_color
&route_text_color
: move to the spec and resolve #59 at the same time.trips.txt
trip_headsign
: "do not" needs to be rephrased as a suggestion "should not"stop_times.txt
shape_dist_traveled
: Word "must" is not suitable for best practice recommendation documentation. Remove from BP and include in "special cases" tutorial section.calendar.txt & calendar_dates.txt
calendar.service_name
: Should we really be recommending the use of extensions in the BP? If this is useful, should this field be adopted in the spec?fare_attributes.txt & fare_rules.txt
agency_id
: "should be included" --> move to spec? cc: recommendation to provideagency_id
for single-agency datasetsshapes.txt
Contradictory recommendations. Which one is better? Choose one. IMHO BP is better:
Spec currently reads:
BP reads:
shape_dist_traveled
: “Must be provided in both shapes.txt and stop_times.txt if an alignment includes looping or inlining (the vehicle crosses or travels over the same portion of alignment in one trip).” —> move to separate tutorial section for instructional requirements.frequencies.txt
transfers.txt
feed_info.txt
Special cases (branches, lasso routes, loop routes)
- "these cases must be modeled like this"
The text was updated successfully, but these errors were encountered: