Skip to content

feat: trip vs shape distance validation#1553

Merged
cka-y merged 5 commits intomasterfrom
feat/163
Sep 25, 2023
Merged

feat: trip vs shape distance validation#1553
cka-y merged 5 commits intomasterfrom
feat/163

Conversation

@cka-y
Copy link
Copy Markdown
Contributor

@cka-y cka-y commented Aug 7, 2023

Summary:

Created trip_distance_exceeds_shape_distancenotice when the max value shapeDistTraveled of a trip is greater than the max value of shapeDistTraveledfor its associated shape.

Closes #163

Expected behavior:

Screenshot 2023-08-07 at 1 10 57 AM

Please make sure these boxes are checked before submitting your pull request - thanks!

@cka-y
Copy link
Copy Markdown
Contributor Author

cka-y commented Aug 7, 2023

@emmambd could you confirm what fields should be present in the notice's details?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 8, 2023

❌ Invalid acceptance test.
New Errors: 0 out of 1439 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1439 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 107 out of 1439 datasets (~7%) are invalid due to code change, which is above the provided threshold of 1%.
Dropped Warnings: 0 out of 1439 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1439 sources (~0 %) are corrupted.
Commit: 712af2a
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

@emmambd
Copy link
Copy Markdown
Contributor

emmambd commented Aug 8, 2023

@cka-y List of fields is correct! This is enough to cross reference the error and find what the problem is.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 9, 2023

❌ Invalid acceptance test.
New Errors: 107 out of 1439 datasets (~7%) are invalid due to code change, which is above the provided threshold of 1%.
Dropped Errors: 0 out of 1439 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1439 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 1 out of 1439 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1439 sources (~0 %) are corrupted.
Commit: 014790d
Download the full acceptance test report here (report will disappear after 90 days).
❌ Invalid acceptance test.

Copy link
Copy Markdown
Contributor

@qcdyx qcdyx left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@emmambd emmambd left a comment

Choose a reason for hiding this comment

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

After doing some spot checks and looking at the report, this looks good to me! Noting that the acceptance tests generate errors for 7% of feeds in the Mobility Database which is above the recommended threshold, but this rule does align with the community consensus from https://github.com/google/transit/pull/380/files. @cka-y One question, not a blocker: any obvious reason why the list of feeds in the acceptance report is slightly different from the one in the original Python analysis report?

Copy link
Copy Markdown
Member

@davidgamez davidgamez left a comment

Choose a reason for hiding this comment

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

It's nicely done!

import org.mobilitydata.gtfsvalidator.table.*;

@RunWith(JUnit4.class)
public class TripAndShapeDistanceValidatorTest {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great addition!

@cka-y cka-y merged commit 176e654 into master Sep 25, 2023
@cka-y cka-y deleted the feat/163 branch September 25, 2023 11:36
@cka-y
Copy link
Copy Markdown
Contributor Author

cka-y commented Sep 25, 2023

@emmambd seems like the script was using the producer URL i.e. current version of the dataset as opposed to the action that uses a persisted version of the dataset. I'm going to keep an eye on this but I think it's safe to merge atm.

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.

Implement trip distance vs. shape verification (GTFS rule)

4 participants