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.

Sign up for GitHub

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

Jump to bottom

Add a list of validators that meet the official GTFS requirements #1315

Closed
wants to merge 4 commits into from
Closed

Add a list of validators that meet the official GTFS requirements #1315

wants to merge 4 commits into from

Conversation

aababilov
Copy link
Collaborator

@aababilov aababilov commented Jan 19, 2023 •
edited by asvechnikov2
Loading

  • BlockTripsWithConsistentTypeValidator
    • Validates that trips that belong to the same block have consistent route types (e.g., bus does not transfer to rail).
    • Generated notice: BlockTripsWithInconsistentRouteTypesNotice.
  • FareAttributeAgencyIdValidator
    • Checks that fare_attributes.agency_id field is defined for every row if there is more than 1 agency in the feed.
    • Generated notice: MissingRequiredFieldNotice.
  • FeedHasLanguageValidator
    • Checks that a feed has language defined in either feed_info.feed_lang or agency.agency_lang. Note that feed_lang is a required field: if there is a feed_info entity, it must have feed_lang set. agency_lang, instead, is optional. The recommended way is to provide feed_lang and omit agency_lang.
    • Generated notice: FeedHasNoLanguageNotice.
  • StationUsageValidator
    • Checks that every station (location_type=1) in stops.txt has child platforms ( location_type=0). Note that a station that has child entrances (location_type=2) or generic nodes (location_type=3) is also reported.
    • Generated notice: StationWithoutPlatformsNotice.
  • StopLatLngRequiredValidator
    • Checks that conditionally required fields stop_lat and stop_lon are set depending on location_type.
      • stop_lat and stop_lon are required for stations (location_type=0), stops (location_type=1) and entrances (location_type=2) and optional for other types.
    • Generated notice: MissingRequiredFieldError.
  • StopTimeInSequenceOrderValidator
    • Validates that fields arrival_time, departure_time and shape_dist_traveled are non-decreasing along each trip.
    • Generated notices:
      • TripWithOutOfOrderArrivalTimeNotice
      • TripWithOutOfOrderDepartureTimeNotice
      • TripWithOutOfOrderShapeDistTraveledNotice
  • TransferFrequencyBasedTripValidator
    • Validates that trip-to-trip transfers do not involve frequency-based trips.
    • Generated notice: TransferForFrequencyBasedTripNotice.
  • TripShapeDistTraveledValidator
    • Validates that trips specify stop_times.shape_dist_traveled values properly. Please see generated notices for details.
    • Generated notices:
      • TripWithShapeDistTraveledButNoShapeNotice
      • TripWithShapeDistTraveledButNoShapeDistancesNotice
      • TripWithPartialShapeDistTraveledNotice

@aababilov aababilov force-pushed the many-validators branch 2 times, most recently from ec85aba to a35c231 Compare January 19, 2023 02:58
@aababilov
Add a list of validators that meet the official GTFS requirements
f85c737 
* BlockTripsWithConsistentTypeValidator
* FareAttributeAgencyIdValidator
* FeedHasLanguageValidator
* StationUsageValidator
* StopNameLatLngRequiredValidator
* StopTimeInSequenceOrderValidator
* TransferFrequencyBasedTripValidator
* TripShapeDistTraveledValidator
asvechnikov2
asvechnikov2 approved these changes Jan 25, 2023
View reviewed changes
Copy link
Collaborator

@asvechnikov2 asvechnikov2 left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.

Hi @isabelle-dr, could you please take a look at the proposed validations if they look good? Do you know if there are any open issues that could be closed by this change?

...ain/java/org/mobilitydata/gtfsvalidator/validator/BlockTripsWithConsistentTypeValidator.java Outdated Show resolved Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FeedHasLanguageValidator.java Outdated Show resolved Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/StationUsageValidator.java Outdated Show resolved Hide resolved
.../src/main/java/org/mobilitydata/gtfsvalidator/validator/StopNameLatLngRequiredValidator.java Outdated Show resolved Hide resolved
...src/main/java/org/mobilitydata/gtfsvalidator/validator/StopTimeInSequenceOrderValidator.java Outdated Show resolved Hide resolved
.../main/java/org/mobilitydata/gtfsvalidator/validator/TransferFrequencyBasedTripValidator.java Outdated Show resolved Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TripShapeDistTraveledValidator.java Outdated Show resolved Hide resolved
@aababilov
Update comments and remove validation of the required stop_name
7da9c4e
@aababilov
Remove StopTimeInSequenceOrderValidator
37228f9 
It partially overlaps with StopTimeIncreasingDistanceValidator, so we
will move it to a separate pull request.
@isabelle-dr
Copy link
Contributor

Thanks for the ping @asvechnikov2! (and.. wow @aababilov 🎉).
I will have a look this week, looking forward to it 😊

@bdferris-v2
Copy link
Collaborator

The LGTM. However, you do need to update RULES.md with the new validation notices you've added.

@bdferris-v2
Copy link
Collaborator

I'd also call out that issue #1304 is probably still blocking us from getting a clean acceptance test run.

briandonahue
briandonahue reviewed Feb 8, 2023
View reviewed changes
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidator.java
public void validate(NoticeContainer noticeContainer) {
if (agencyTable.entityCount() <= 1) {
// fare_attributes.agency_id is not required when there is a single agency.
return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@isabelle-dr @aababilov I've added similar logic as part of #1318 and added a best practice MissingRecommendedFieldNotice warning to recommend that agency_id be included even when there is only one agency.

@briandonahue briandonahue mentioned this pull request Feb 8, 2023
Merged
4 tasks
@isabelle-dr isabelle-dr mentioned this pull request Feb 9, 2023
Closed
@isabelle-dr
Copy link
Contributor

isabelle-dr commented Feb 9, 2023 •
edited
Loading

Hello! Sorry it took a while. A few things here:

  1. FeedHasLanguageValidator
    Is this mentioned in the spec somewhere? I couldn't find it. Adding this to the spec looks like a breaking change. Maybe something to add in the best Practice and flag as a warning here? Where is this coming from?
  2. StationUsageValidator
    Reading the spec, I think stations without any children are okay? Please tell me if you think otherwise.
  3. BlockTripsWithConsistentTypeValidator, TransferFrequencyBasedTripValidator and TripShapeDistTraveledValidator
    These new validators make a lot of sense! That being said, so explicit spec mention. I suggest we first amend the spec and then merge them. Happy to propose the changes.
  4. StopLatLngRequiredValidator-> it looks like it has been solved in fix: generate errors when missing lat/long for stops, stations, entrances #1321 by @briandonahue
  5. FareAttributeAgencyIdValidator -> LGTM
  6. StopTimeInSequenceOrderValidator -> it looks like it's also in Validate that arrival_time, departure_time and shape_dist_traveled are non-decreasing along each trip #1319

@isabelle-dr
Copy link
Contributor

I suggest we split this PR between what's explicitly mentioned in spec VS what is not.
What is explicitly mentioned: we just need the documentation update and we're good to merge.
What isn't: add the clear statements in the spec itself, and then merge

@isabelle-dr isabelle-dr linked an issue Feb 10, 2023 that may be closed by this pull request
Add a rule that checks the presence of stop_lat & stop_lon in stops.txt #1180
Closed
@isabelle-dr isabelle-dr mentioned this pull request Feb 10, 2023
Merged
4 tasks
@aababilov aababilov closed this Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@briandonahue briandonahue briandonahue left review comments

@asvechnikov2 asvechnikov2 asvechnikov2 approved these changes

@isabelle-dr isabelle-dr Awaiting requested review from isabelle-dr

Assignees
No one assigned
Labels
None yet
Projects
Schedule Validator Q2 backlog
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add a rule that checks the presence of stop_lat & stop_lon in stops.txt
5 participants
@aababilov @isabelle-dr @bdferris-v2 @briandonahue @asvechnikov2