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: changes + added documentation to features #1844

Merged
merged 7 commits into from
Oct 1, 2024
Merged

feat: changes + added documentation to features #1844

merged 7 commits into from
Oct 1, 2024

Conversation

cka-y
Copy link
Contributor

@cka-y cka-y commented Sep 23, 2024

Summary:

  • Updated features based on the table description
  • Added documentation links to the HTML report for each feature

Expected Behavior:

  • Ensures consistent naming conventions between the HTML report and the official documentation.
  • Each feature listed in the report now includes a clickable link that directs users to the corresponding section in the official documentation
Screenshot 2024-09-23 at 4 26 44 PM

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

📝 Acceptance Test Report

📋 Summary

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

📊 Notices Comparison

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

No changes were detected due to the code change.

Dropped Errors (0 out of 1575 datasets, ~0%) ✅

No changes were detected due to the code change.

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 Check

0 out of 1575 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.06 4.11 ⬆️+0.05
Median -- 1.41 1.46 ⬆️+0.05
Standard Deviation -- 11.70 11.63 ⬇️-0.07
Minimum in References Reports us-florida-citrus-county-transit-gtfs-630 0.51 0.61 ⬆️+0.10
Maximum in Reference Reports gb-unknown-uk-aggregate-feed-gtfs-2014 294.52 290.99 ⬇️-3.53
Minimum in Latest Reports us-oregon-high-desert-point-gtfs-636 0.52 0.53 ⬆️+0.01
Maximum in Latest Reports gb-unknown-uk-aggregate-feed-gtfs-2014 294.52 290.99 ⬇️-3.53

@cka-y cka-y requested a review from jcpitre September 24, 2024 18:23
@emmambd
Copy link
Contributor

emmambd commented Sep 24, 2024

@cka-y Looks great! Super minor feedback:

Capitalize "In-Station Traversal Time"
Capitalize "Zone-Based Demand Responsive Services" and center text
Screenshot 2024-09-24 at 2 58 46 PM

@tzujenchanmbd @Sergiodero Do you know of specific feeds in the production list that have Predefined Routes with Deviation or Fixed Stops Demand Responsive Service to test with? I'm having some trouble identifying them.

Copy link
Contributor

📝 Acceptance Test Report

📋 Summary

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

📊 Notices Comparison

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

No changes were detected due to the code change.

Dropped Errors (0 out of 1575 datasets, ~0%) ✅

No changes were detected due to the code change.

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 Check

0 out of 1575 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.05 4.09 ⬆️+0.04
Median -- 1.42 1.46 ⬆️+0.04
Standard Deviation -- 11.49 11.58 ⬆️+0.09
Minimum in References Reports us-massachusetts-massachusetts-area-express-max-gtfs-431 0.48 0.61 ⬆️+0.13
Maximum in Reference Reports gb-unknown-uk-aggregate-feed-gtfs-2014 290.59 293.41 ⬆️+2.83
Minimum in Latest Reports ph-unknown-hm-transport-inc-and-robinsons-malls-gtfs-1105 0.58 0.53 ⬇️-0.05
Maximum in Latest Reports gb-unknown-uk-aggregate-feed-gtfs-2014 290.59 293.41 ⬆️+2.83

Copy link
Contributor

@jcpitre jcpitre left a comment

Choose a reason for hiding this comment

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

LGTM!

@cka-y
Copy link
Contributor Author

cka-y commented Sep 25, 2024

✅ done @emmambd

@tzujenchanmbd
Copy link

@emmambd
Feeds for test:
Fixed Stops - https://data.trilliumtransit.com/gtfs/islandtransit-wa-us/islandtransit-wa-us--flex-v2-TEST.zip
Predefined Routes with Deviation - https://data.trilliumtransit.com/gtfs/browncounty-mn-us/browncounty-mn-us--flex-v2.zip

@emmambd
Copy link
Contributor

emmambd commented Sep 25, 2024

@tzujenchanmbd It looks like the Predefined Routes doesn't trigger for the feed you provided. I think it's because the logic for it from #1776 was:

Deviated Fixed Route: At least a trip in stop_times references location_id AND stop_id AND arrival_time AND departure_time

It looks like it's either location_id or stop_id from the data. Should the rule instead be:

Two trips:

At least one trip in stop_times references location_id AND one trip in stop_times references stop_id, arrival_time and departure_time

Copy link
Contributor

📝 Acceptance Test Report

📋 Summary

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

📊 Notices Comparison

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

No changes were detected due to the code change.

Dropped Errors (0 out of 1575 datasets, ~0%) ✅

No changes were detected due to the code change.

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 Check

0 out of 1575 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.07 4.20 ⬆️+0.13
Median -- 1.42 1.50 ⬆️+0.08
Standard Deviation -- 11.74 11.67 ⬇️-0.07
Minimum in References Reports us-massachusetts-massachusetts-area-express-max-gtfs-431 0.53 0.64 ⬆️+0.12
Maximum in Reference Reports gb-unknown-uk-aggregate-feed-gtfs-2014 303.47 297.37 ⬇️-6.10
Minimum in Latest Reports us-california-flex-v2-developer-test-feed-3-gtfs-1819 0.61 0.56 ⬇️-0.06
Maximum in Latest Reports gb-unknown-uk-aggregate-feed-gtfs-2014 303.47 297.37 ⬇️-6.10

@tzujenchanmbd
Copy link

@emmambd The rule should still check within 1 trip instead of 2.


Screenshot 2024-09-25 at 12 00 45 PM

It seems the trip t_5374696_b_77497_tn_0 does include these 4 field values, but not in the same entry. Perhaps the current logic is checking the same entry?

@emmambd
Copy link
Contributor

emmambd commented Oct 1, 2024

I'll add a new issue for the problem with Predefined Routes.

@cka-y, last comment

@cka-y
Copy link
Contributor Author

cka-y commented Oct 1, 2024

done here @emmambd

@cka-y cka-y merged commit be675f3 into master Oct 1, 2024
333 checks passed
@cka-y cka-y deleted the feat/1679 branch October 1, 2024 15:42
Copy link
Contributor

github-actions bot commented Oct 1, 2024

📝 Acceptance Test Report

📋 Summary

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

📊 Notices Comparison

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

No changes were detected due to the code change.

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.06 4.11 ⬆️+0.06
Median -- 1.42 1.48 ⬆️+0.06
Standard Deviation -- 11.51 11.59 ⬆️+0.08
Minimum in References Reports ph-unknown-hm-transport-inc-and-robinsons-malls-gtfs-1105 0.51 0.51 ⬇️-0.00
Maximum in Reference Reports gb-unknown-uk-aggregate-feed-gtfs-2014 293.68 298.51 ⬆️+4.83
Minimum in Latest Reports ph-unknown-hm-transport-inc-and-robinsons-malls-gtfs-1105 0.51 0.51 ⬇️-0.00
Maximum in Latest Reports gb-unknown-uk-aggregate-feed-gtfs-2014 293.68 298.51 ⬆️+4.83

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.

Changes to certain GTFS features
4 participants