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.

Sign up for GitHub

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

Jump to bottom

Remove legacy bike access mapping #6248

Merged
merged 3 commits into from
Nov 20, 2024
Merged

Remove legacy bike access mapping #6248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1287304
Reapply "Remove legacy bike access mapping"
leonardehrenfried Nov 12, 2024
61ca183
Remove unused variable
leonardehrenfried Nov 12, 2024
d2cf56f
Change trip_bikes_allowed, route_bikes_allowed in simple GTFS feed
leonardehrenfried Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 9 additions & 50 deletions application/src/main/java/org/opentripplanner/gtfs/mapping/BikeAccessMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,64 +5,23 @@
import org.opentripplanner.transit.model.network.BikeAccess;

/**
* Model bike access for GTFS trips.
* <p>
* The GTFS bike extensions is originally discussed at: https://groups.google.com/d/msg/gtfs-changes/QqaGOuNmG7o/xyqORy-T4y0J
* <p>
* It proposes "route_bikes_allowed" in routes.txt and "trip_bikes_allowed" in trips.txt with the
* following semantics:
* <p>
* 2: bikes allowed<br> 1: no bikes allowed<br> 0: no information (same as field omitted)<br>
* <p>
* The values in trips.txt override the values in routes.txt.
* <p>
* An alternative proposal is discussed in: https://groups.google.com/d/msg/gtfs-changes/rEiSeKNc4cs/gTTnQ_yXtPgJ
* <p>
* Here, the field "bikes_allowed" is used in both routes.txt and trip.txt with the following
* semantics:
* <p>
* 2: no bikes allowed<br> 1: bikes allowed<br> 0: no information (same as field omitted)<br>
* <p>
* Here, the 0,1,2 semantics have been changed to match the convention used in the
* "wheelchair_accessible" field in trips.txt.
* <p>
* A number of feeds are still using the original proposal and a number of feeds have been updated
* to use the new proposal. For now, we support both, using "bikes_allowed" if specified and then
* "trip_bikes_allowed".
* Model bike access for GTFS trips by using the bikes_allowed fields from route and trip.
*/
class BikeAccessMapper {

public static BikeAccess mapForTrip(Trip rhs) {
//noinspection deprecation
return mapValues(rhs.getBikesAllowed(), rhs.getTripBikesAllowed());
return mapValues(rhs.getBikesAllowed());
}

public static BikeAccess mapForRoute(Route rhs) {
//noinspection deprecation
return mapValues(rhs.getBikesAllowed(), rhs.getRouteBikesAllowed());
return mapValues(rhs.getBikesAllowed());
}
Comment on lines 16 to 18
Copy link
Contributor

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

Copy link
Member Author

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.

Copy link
Member Author

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.

Copy link
Member

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.


private static BikeAccess mapValues(int bikesAllowed, int legacyBikesAllowed) {
if (bikesAllowed != 0) {
switch (bikesAllowed) {
case 1:
return BikeAccess.ALLOWED;
case 2:
return BikeAccess.NOT_ALLOWED;
default:
return BikeAccess.UNKNOWN;
}
} else if (legacyBikesAllowed != 0) {
switch (legacyBikesAllowed) {
case 1:
return BikeAccess.NOT_ALLOWED;
case 2:
return BikeAccess.ALLOWED;
default:
return BikeAccess.UNKNOWN;
}
}

return BikeAccess.UNKNOWN;
private static BikeAccess mapValues(int bikesAllowed) {
return switch (bikesAllowed) {
case 1 -> BikeAccess.ALLOWED;
case 2 -> BikeAccess.NOT_ALLOWED;
default -> BikeAccess.UNKNOWN;
};
}
}
39 changes: 0 additions & 39 deletions application/src/test/java/org/opentripplanner/gtfs/mapping/BikeAccessMapperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ public class BikeAccessMapperTest {

private static final int BIKES_ALLOWED = 1;
private static final int BIKES_NOT_ALLOWED = 2;
private static final int TRIP_BIKES_ALLOWED = 2;
private static final int TRIP_BIKES_NOT_ALLOWED = 1;
private static final int ROUTE_BIKES_ALLOWED = 2;
private static final int ROUTE_BIKES_NOT_ALLOWED = 1;

@Test
public void testTripProvidedValues() {
Expand All @@ -28,25 +24,12 @@ public void testTripProvidedValues() {
assertEquals(BikeAccess.NOT_ALLOWED, BikeAccessMapper.mapForTrip(trip));
}

@Test
public void testLegacyTripProvidedValues() {
Trip trip = new Trip();
assertEquals(BikeAccess.UNKNOWN, BikeAccessMapper.mapForTrip(trip));

trip.setTripBikesAllowed(TRIP_BIKES_ALLOWED);
assertEquals(BikeAccess.ALLOWED, BikeAccessMapper.mapForTrip(trip));

trip.setTripBikesAllowed(TRIP_BIKES_NOT_ALLOWED);
assertEquals(BikeAccess.NOT_ALLOWED, BikeAccessMapper.mapForTrip(trip));
}

@Test
public void testTripProvidedValuesPrecedence() {
Trip trip = new Trip();
assertEquals(BikeAccess.UNKNOWN, BikeAccessMapper.mapForTrip(trip));

trip.setBikesAllowed(BIKES_ALLOWED);
trip.setTripBikesAllowed(TRIP_BIKES_NOT_ALLOWED);
assertEquals(BikeAccess.ALLOWED, BikeAccessMapper.mapForTrip(trip));
}

Expand All @@ -61,26 +44,4 @@ public void testRouteProvidedValues() {
route.setBikesAllowed(BIKES_NOT_ALLOWED);
assertEquals(BikeAccess.NOT_ALLOWED, BikeAccessMapper.mapForRoute(route));
}

@Test
public void testLegacyRouteProvidedValues() {
Route route = new Route();
assertEquals(BikeAccess.UNKNOWN, BikeAccessMapper.mapForRoute(route));

route.setRouteBikesAllowed(ROUTE_BIKES_ALLOWED);
assertEquals(BikeAccess.ALLOWED, BikeAccessMapper.mapForRoute(route));

route.setRouteBikesAllowed(ROUTE_BIKES_NOT_ALLOWED);
assertEquals(BikeAccess.NOT_ALLOWED, BikeAccessMapper.mapForRoute(route));
}

@Test
public void testRouteProvidedValuesPrecedence() {
Route route = new Route();
assertEquals(BikeAccess.UNKNOWN, BikeAccessMapper.mapForRoute(route));

route.setBikesAllowed(BIKES_ALLOWED);
route.setRouteBikesAllowed(ROUTE_BIKES_NOT_ALLOWED);
assertEquals(BikeAccess.ALLOWED, BikeAccessMapper.mapForRoute(route));
}
}
8 changes: 4 additions & 4 deletions application/src/test/resources/gtfs/simple/routes.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
route_id,route_short_name,route_long_name,route_type,route_bikes_allowed
1,1,1,3,2
2,2,2,3,1
route_id,route_short_name,route_long_name,route_type,bikes_allowed
1,1,1,3,1
2,2,2,3,2
3,3,3,3,
4,4,4,7,
5,5,5,2,
6,6,6,2,
7,7,7,2,
8,8,8,3,
9,9,9,2,2
9,9,9,2,1
10,10,10,2,
11,11,11,2,
12,12,12,2,
Expand Down
8 changes: 4 additions & 4 deletions application/src/test/resources/gtfs/simple/trips.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
route_id,service_id,trip_id,shape_id,block_id,wheelchair_accessible,trip_bikes_allowed,direction_id,trip_headsign
route_id,service_id,trip_id,shape_id,block_id,wheelchair_accessible,bikes_allowed,direction_id,trip_headsign
1,alldays,1.1,,,1,,,foo
1,alldays,1.2,,,1,,,foo
1,alldays,1.3,,,1,,,foo
2,alldays,2.1,,,0,2,,foo
2,alldays,2.2,,,0,2,,foo
2,alldays,2.1,,,0,1,,foo
2,alldays,2.2,,,0,1,,foo
3,alldays,3.1,,,1,,,foo
3,alldays,3.2,,,1,,,foo
4,weekdays,4.1,4,,,,,foo
Expand All @@ -15,7 +15,7 @@ route_id,service_id,trip_id,shape_id,block_id,wheelchair_accessible,trip_bikes_a
6,alldays,6.2,,block.2,,,,foo
7,alldays,7.2,,block.2,,,,foo
8,alldays,8.1,,block.2,,,,foo
9,alldays,9.1,,,,1,,foo
9,alldays,9.1,,,,2,,foo
10,alldays,10.1,,,,,,foo
10,alldays,10.2,,,,,,foo
10,alldays,10.3,,,,,,foo
Expand Down
Loading