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.

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: new booking_rules.txt validation notices #1866

Merged
merged 4 commits into from
Oct 3, 2024

Conversation

cka-y
Copy link
Contributor

@cka-y cka-y commented Oct 2, 2024

Summary:

This pull request enhances the BookingRulesEntityValidator by adding new validation checks and corresponding notices. It also updates the test cases to cover these new validations.

Added missing_prior_notice_durantion_min, missing_prior_day_bokking_field_value, forbidden_prior_notice_start_time and missing_prior_notice_start_time notices to the validator

image

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

  • Run the unit tests with gradle test to make sure you didn't break anything
  • Add or update any needed documentation to the repo
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues
  • Include screenshot(s) showing how this pull request works and fixes the issue(s)

Copy link
Contributor

github-actions bot commented Oct 2, 2024

📝 Acceptance Test Report

📋 Summary

✅ The rule acceptance has passed for commit de49dd1
Download the full acceptance test report here (report will disappear after 90 days).

📊 Notices Comparison

New Errors (6 out of 1588 datasets, ~0%) ✅

Details of new errors due to code change, which is less than the provided threshold of 1%.

Dataset Notice Code
us-colorado-eco-transit-gtfs-2045 forbidden_prior_notice_start_time
us-colorado-gunnison-valley-rta-gtfs-2048 forbidden_prior_notice_start_time
us-colorado-prowers-area-transit-gtfs-2049 forbidden_prior_notice_start_time
us-colorado-avon-transit-gtfs-2040 missing_prior_day_booking_field_value
us-colorado-eco-transit-gtfs-2045 missing_prior_day_booking_field_value
us-colorado-envida-gtfs-2044 missing_prior_day_booking_field_value
us-colorado-estes-transit-gtfs-2047 missing_prior_day_booking_field_value
us-colorado-prowers-area-transit-gtfs-2049 missing_prior_day_booking_field_value
Dropped Errors (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

New Warnings (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

Dropped Warnings (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

🛡️ Corruption Check

0 out of 1588 sources (~0 %) are corrupted.

⏱️ Performance Assessment

📈 Validation Time

Assess the performance in terms of seconds taken for the validation process.

Time Metric Dataset ID Reference (s) Latest (s) Difference (s)
Average -- 4.44 4.27 ⬇️-0.18
Median -- 1.43 1.48 ⬆️+0.05
Standard Deviation -- 14.52 12.26 ⬇️-2.26
Minimum in References Reports us-michigan-detroit-people-mover-gtfs-417 0.52 0.61 ⬆️+0.09
Maximum in Reference Reports gb-unknown-uk-aggregate-feed-gtfs-2014 289.47 294.15 ⬆️+4.67
Minimum in Latest Reports us-california-city-of-wasco-gtfs-1788 0.56 0.56 ⬇️-0.01
Maximum in Latest Reports gb-unknown-uk-aggregate-feed-gtfs-2014 289.47 294.15 ⬆️+4.67

Copy link
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.

LGTM

@tzujenchanmbd
Copy link

@cka-y 1 question for the missing_prior_notice_start_time notice in the screenshot -

The notice should be triggered when there is no prior_notice_start_time value AND prior_notice_start_day value exists. It's odd that we still have a priorNoticeStartTime value 00:00:00 there?

Copy link
Contributor

github-actions bot commented Oct 3, 2024

📝 Acceptance Test Report

📋 Summary

✅ The rule acceptance has passed for commit 738e3fb
Download the full acceptance test report here (report will disappear after 90 days).

📊 Notices Comparison

New Errors (6 out of 1588 datasets, ~0%) ✅

Details of new errors due to code change, which is less than the provided threshold of 1%.

Dataset Notice Code
us-colorado-eco-transit-gtfs-2045 forbidden_prior_notice_start_time
us-colorado-gunnison-valley-rta-gtfs-2048 forbidden_prior_notice_start_time
us-colorado-prowers-area-transit-gtfs-2049 forbidden_prior_notice_start_time
us-colorado-avon-transit-gtfs-2040 missing_prior_day_booking_field_value
us-colorado-eco-transit-gtfs-2045 missing_prior_day_booking_field_value
us-colorado-envida-gtfs-2044 missing_prior_day_booking_field_value
us-colorado-estes-transit-gtfs-2047 missing_prior_day_booking_field_value
us-colorado-prowers-area-transit-gtfs-2049 missing_prior_day_booking_field_value
Dropped Errors (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

New Warnings (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

Dropped Warnings (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

🛡️ Corruption Check

0 out of 1588 sources (~0 %) are corrupted.

⏱️ Performance Assessment

📈 Validation Time

Assess the performance in terms of seconds taken for the validation process.

Time Metric Dataset ID Reference (s) Latest (s) Difference (s)
Average -- 4.03 4.11 ⬆️+0.08
Median -- 1.41 1.49 ⬆️+0.08
Standard Deviation -- 11.40 11.44 ⬆️+0.04
Minimum in References Reports us-california-city-of-wasco-gtfs-1788 0.50 0.52 ⬆️+0.02
Maximum in Reference Reports gb-unknown-uk-aggregate-feed-gtfs-2014 291.32 295.24 ⬆️+3.91
Minimum in Latest Reports us-california-city-of-wasco-gtfs-1788 0.50 0.52 ⬆️+0.02
Maximum in Latest Reports gb-unknown-uk-aggregate-feed-gtfs-2014 291.32 295.24 ⬆️+3.91

@cka-y
Copy link
Contributor Author

cka-y commented Oct 3, 2024

@tzujenchanmbd Great catch! The 00:00:00 value is the default for null GtfsTime. I’ve replaced priorNoticeStartTime with priorNoticeStartDay in the notice and updated the screenshot accordingly. Let me know what you think!

Copy link

@tzujenchanmbd tzujenchanmbd left a comment

Choose a reason for hiding this comment

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

Thanks @cka-y !
LGTM on notice name, field displayed, and description.

@cka-y cka-y merged commit e3b9f3d into master Oct 3, 2024
334 checks passed
@cka-y cka-y deleted the feat/more-booking_rules-notices branch October 3, 2024 15:58
Copy link
Contributor

github-actions bot commented Oct 3, 2024

📝 Acceptance Test Report

📋 Summary

✅ The rule acceptance has passed for commit 4f311d9
Download the full acceptance test report here (report will disappear after 90 days).

📊 Notices Comparison

New Errors (6 out of 1588 datasets, ~0%) ✅

Details of new errors due to code change, which is less than the provided threshold of 1%.

Dataset Notice Code
us-colorado-eco-transit-gtfs-2045 forbidden_prior_notice_start_time
us-colorado-gunnison-valley-rta-gtfs-2048 forbidden_prior_notice_start_time
us-colorado-prowers-area-transit-gtfs-2049 forbidden_prior_notice_start_time
us-colorado-avon-transit-gtfs-2040 missing_prior_day_booking_field_value
us-colorado-eco-transit-gtfs-2045 missing_prior_day_booking_field_value
us-colorado-envida-gtfs-2044 missing_prior_day_booking_field_value
us-colorado-estes-transit-gtfs-2047 missing_prior_day_booking_field_value
us-colorado-prowers-area-transit-gtfs-2049 missing_prior_day_booking_field_value
Dropped Errors (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

New Warnings (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

Dropped Warnings (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

🛡️ Corruption Check

0 out of 1588 sources (~0 %) are corrupted.

⏱️ Performance Assessment

📈 Validation Time

Assess the performance in terms of seconds taken for the validation process.

Time Metric Dataset ID Reference (s) Latest (s) Difference (s)
Average -- 4.01 4.07 ⬆️+0.06
Median -- 1.42 1.47 ⬆️+0.05
Standard Deviation -- 11.36 11.46 ⬆️+0.10
Minimum in References Reports us-california-catalina-express-gtfs-299 0.50 0.60 ⬆️+0.10
Maximum in Reference Reports gb-unknown-uk-aggregate-feed-gtfs-2014 288.57 296.03 ⬆️+7.47
Minimum in Latest Reports us-massachusetts-massachusetts-area-express-max-gtfs-431 0.54 0.52 ⬇️-0.02
Maximum in Latest Reports gb-unknown-uk-aggregate-feed-gtfs-2014 288.57 296.03 ⬆️+7.47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants