-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Remove legacy bike access mapping #6248
Remove legacy bike access mapping #6248
Conversation
This reverts commit 26a654f.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6248 +/- ##
=============================================
- Coverage 69.73% 69.71% -0.03%
- Complexity 17680 17693 +13
=============================================
Files 2007 2008 +1
Lines 75771 75827 +56
Branches 7754 7762 +8
=============================================
+ Hits 52842 52864 +22
- Misses 20217 20252 +35
+ Partials 2712 2711 -1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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.
The simple test GTFS contains both route_bikes_allowed
and trip_bikes_allowed
columns. Should those be removed, replaced with the standard versions?
route_id,route_short_name,route_long_name,route_type,route_bikes_allowed |
route_id,service_id,trip_id,shape_id,block_id,wheelchair_accessible,trip_bikes_allowed,direction_id,trip_headsign |
public static BikeAccess mapForRoute(Route rhs) { | ||
//noinspection deprecation | ||
return mapValues(rhs.getBikesAllowed(), rhs.getRouteBikesAllowed()); | ||
return mapValues(rhs.getBikesAllowed()); | ||
} |
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.
bikes_allowed
only seems to be present in trips.txt
. Should the route based handling also be removed?
https://gtfs.org/documentation/schedule/reference/#routestxt
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.
You're totally right - this is actually not part of the spec.
I think I want to ask around if this is really not in use by the current users (HSL, IBI, RealCity) and then open another PR.
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.
I would also have a knock-on effect on the Route
type so that would also require more discussion.
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.
Pretty sure we don't have the bikes_allowed information in routes.txt in any of our data sets.
Good catch. Updated. |
Summary
This removes the parsing of legacy
bikes_allowed
information. The fieldstrip_bikes_allowed
androute_bikes_allowed
were discussed early in the GTFS spec process but have never made it into the specification.It has been deprecated in onebusaway-gtfs for many years and removed in the latest version.
Issue
None.
Unit tests
Existing ones updated.