-
Notifications
You must be signed in to change notification settings - Fork 101
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
feat: update missing_trip_edge
for flex feed
#1843
Conversation
📝 Acceptance Test Report📋 Summary❌ The rule acceptance test has failed for commit 82b9940 📊 Notices ComparisonNew Errors (0 out of 1575 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (19 out of 1575 datasets, ~1%) ❌Details of new errors due to code change, which is above the provided threshold of 1%.
New Warnings (0 out of 1575 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1575 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1575 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
|
@cka-y Cross referencing with the original PR that added these errors because stop_times.txt was finally parseable for Flex feeds, this acceptance test update looks right! The only one confusing me is cc @tzujenchanmbd and @Sergiodero as reference for how we review the acceptance tests :) |
📝 Acceptance Test Report📋 Summary❌ The rule acceptance test has failed for commit 82b9940 📊 Notices ComparisonNew Errors (0 out of 1575 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (18 out of 1575 datasets, ~1%) ❌Details of new errors due to code change, which is above the provided threshold of 1%.
New Warnings (0 out of 1575 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1575 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1575 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
|
@emmambd seems like there was a problem with the acceptance tests. Without updating the code, I ran them again and everything seems to work as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary:
This update modifies the
missing_trip_edge
notice to ensure it is not triggered if theGtfsStopTime
entity has either thestartPickupDropOffWindow
orendPickupDropOffWindow
column defined.Expected Behavior:
The
missing_trip_edge
notice will no longer be triggered in cases where one of the above columns is present. For example, using this dataset, the notice is no longer triggered, as shown below:Please make sure these boxes are checked before submitting your pull request - thanks!
gradle test
to make sure you didn't break anything