Merge pull request #5341 from leonardehrenfried/ban-discouraged

Remove banDiscouragedCycling and banDiscouragedWalking
opentripplanner · Sep 14, 2023 · b5cbc95 · b5cbc95
2 parents 7135319 + 00ce283
commit b5cbc95
Show file tree

Hide file tree

Showing 12 changed files with 50 additions and 63 deletions.
diff --git a/docs/BuildConfiguration.md b/docs/BuildConfiguration.md
@@ -20,8 +20,6 @@ Sections follow that describe particular settings in more depth.
 | Config Parameter                                                         |     Type    | Summary                                                                                                                                                        |  Req./Opt. | Default Value                     | Since |
 |--------------------------------------------------------------------------|:-----------:|----------------------------------------------------------------------------------------------------------------------------------------------------------------|:----------:|-----------------------------------|:-----:|
 | [areaVisibility](#areaVisibility)                                        |  `boolean`  | Perform visibility calculations.                                                                                                                               | *Optional* | `false`                           |  1.5  |
-| banDiscouragedBiking                                                     |  `boolean`  | Should biking be allowed on OSM ways tagged with `bicycle=discouraged`                                                                                         | *Optional* | `false`                           |  2.0  |
-| banDiscouragedWalking                                                    |  `boolean`  | Should walking be allowed on OSM ways tagged with `foot=discouraged`                                                                                           | *Optional* | `false`                           |  2.0  |
 | [buildReportDir](#buildReportDir)                                        |    `uri`    | URI to the directory where the graph build report should be written to.                                                                                        | *Optional* |                                   |  2.0  |
 | [configVersion](#configVersion)                                          |   `string`  | Deployment version of the *build-config.json*.                                                                                                                 | *Optional* |                                   |  2.1  |
 | [dataImportReport](#dataImportReport)                                    |  `boolean`  | Generate nice HTML report of Graph errors/warnings                                                                                                             | *Optional* | `false`                           |  2.0  |

diff --git a/docs/examples/entur/build-config.json b/docs/examples/entur/build-config.json
@@ -13,8 +13,6 @@
     "islandWithoutStopsMaxSize": 5,
     "islandWithStopsMaxSize": 5
   },
-  "banDiscouragedWalking": false,
-  "banDiscouragedBiking": false,
   "maxTransferDuration": "30m",
   "distanceBetweenElevationSamples": 25,
   "multiThreadElevationCalculations": true,

diff --git a/src/main/java/org/opentripplanner/graph_builder/module/configure/GraphBuilderModules.java b/src/main/java/org/opentripplanner/graph_builder/module/configure/GraphBuilderModules.java
@@ -73,8 +73,6 @@ static OsmModule provideOpenStreetMapModule(
       .withPlatformEntriesLinking(config.platformEntriesLinking)
       .withStaticParkAndRide(config.staticParkAndRide)
       .withStaticBikeParkAndRide(config.staticBikeParkAndRide)
-      .withBanDiscouragedWalking(config.banDiscouragedWalking)
-      .withBanDiscouragedBiking(config.banDiscouragedBiking)
       .withMaxAreaNodes(config.maxAreaNodes)
       .withBoardingAreaRefTags(config.boardingLocationTags)
       .withIssueStore(issueStore)

diff --git a/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmFilter.java b/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmFilter.java
@@ -72,8 +72,6 @@ public static StreetTraversalPermission getPermissionsForEntity(
   public static StreetTraversalPermission getPermissionsForWay(
     OSMWay way,
     StreetTraversalPermission def,
-    boolean banDiscouragedWalking,
-    boolean banDiscouragedBiking,
     DataImportIssueStore issueStore
   ) {
     StreetTraversalPermission permissions = getPermissionsForEntity(way, def);
@@ -103,22 +101,14 @@ public static StreetTraversalPermission getPermissionsForWay(
       permissions = permissions.remove(StreetTraversalPermission.PEDESTRIAN);
     }
 
-    // Check for foot=discouraged, if applicable
-    if (banDiscouragedWalking && way.hasTag("foot") && way.getTag("foot").equals("discouraged")) {
-      permissions = permissions.remove(StreetTraversalPermission.PEDESTRIAN);
-    }
-
     // Compute bike permissions, check consistency.
     boolean forceBikes = false;
     if (way.isBicycleExplicitlyAllowed()) {
       permissions = permissions.add(StreetTraversalPermission.BICYCLE);
       forceBikes = true;
     }
 
-    if (
-      way.isBicycleDismountForced() ||
-      (banDiscouragedBiking && way.hasTag("bicycle") && way.getTag("bicycle").equals("discouraged"))
-    ) {
+    if (way.isBicycleDismountForced()) {
       permissions = permissions.remove(StreetTraversalPermission.BICYCLE);
       if (forceBikes) {
         issueStore.add(new ConflictingBikeTags(way));
@@ -132,7 +122,7 @@ public static StreetTraversalPermission getPermissionsForWay(
     OSMWay way,
     StreetTraversalPermission def
   ) {
-    return getPermissionsForWay(way, def, false, false, DataImportIssueStore.NOOP);
+    return getPermissionsForWay(way, def, DataImportIssueStore.NOOP);
   }
 
   /**

diff --git a/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java b/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java
@@ -250,8 +250,6 @@ private void buildBasicGraph() {
       StreetTraversalPermission permissions = OsmFilter.getPermissionsForWay(
         way,
         wayData.getPermission(),
-        params.banDiscouragedWalking(),
-        params.banDiscouragedBiking(),
         issueStore
       );
       if (!OsmFilter.isWayRoutable(way) || permissions.allowsNothing()) {

diff --git a/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModuleBuilder.java b/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModuleBuilder.java
@@ -23,8 +23,6 @@ public class OsmModuleBuilder {
   private boolean platformEntriesLinking = false;
   private boolean staticParkAndRide = false;
   private boolean staticBikeParkAndRide = false;
-  private boolean banDiscouragedWalking = false;
-  private boolean banDiscouragedBiking = false;
   private int maxAreaNodes;
 
   OsmModuleBuilder(Collection<OsmProvider> providers, Graph graph) {
@@ -67,16 +65,6 @@ public OsmModuleBuilder withStaticBikeParkAndRide(boolean staticBikeParkAndRide)
     return this;
   }
 
-  public OsmModuleBuilder withBanDiscouragedWalking(boolean banDiscouragedWalking) {
-    this.banDiscouragedWalking = banDiscouragedWalking;
-    return this;
-  }
-
-  public OsmModuleBuilder withBanDiscouragedBiking(boolean banDiscouragedBiking) {
-    this.banDiscouragedBiking = banDiscouragedBiking;
-    return this;
-  }
-
   public OsmModuleBuilder withMaxAreaNodes(int maxAreaNodes) {
     this.maxAreaNodes = maxAreaNodes;
     return this;
@@ -94,9 +82,7 @@ public OsmModule build() {
         areaVisibility,
         platformEntriesLinking,
         staticParkAndRide,
-        staticBikeParkAndRide,
-        banDiscouragedWalking,
-        banDiscouragedBiking
+        staticBikeParkAndRide
       )
     );
   }

diff --git a/...java/org/opentripplanner/graph_builder/module/osm/parameters/OsmProcessingParameters.java b/...java/org/opentripplanner/graph_builder/module/osm/parameters/OsmProcessingParameters.java
@@ -13,10 +13,6 @@
  * @param platformEntriesLinking Whether platform entries should be linked
  * @param staticParkAndRide      Whether we should create car P+R stations from OSM data.
  * @param staticBikeParkAndRide  Whether we should create bike P+R stations from OSM data.
- * @param banDiscouragedWalking  Whether ways tagged foot=discouraged should be marked as
- *                               inaccessible.
- * @param banDiscouragedBiking   Whether ways tagged bicycle=discouraged should be marked as
- *                               inaccessible.
  */
 public record OsmProcessingParameters(
   Set<String> boardingAreaRefTags,
@@ -25,9 +21,7 @@ public record OsmProcessingParameters(
   boolean areaVisibility,
   boolean platformEntriesLinking,
   boolean staticParkAndRide,
-  boolean staticBikeParkAndRide,
-  boolean banDiscouragedWalking,
-  boolean banDiscouragedBiking
+  boolean staticBikeParkAndRide
 ) {
   public OsmProcessingParameters {
     boardingAreaRefTags = Set.copyOf(Objects.requireNonNull(boardingAreaRefTags));

diff --git a/src/main/java/org/opentripplanner/openstreetmap/model/OSMWithTags.java b/src/main/java/org/opentripplanner/openstreetmap/model/OSMWithTags.java
@@ -72,12 +72,17 @@ public void addTag(OSMTag tag) {
   /**
    * Adds a tag.
    */
-  public void addTag(String key, String value) {
-    if (key == null || value == null) return;
+  public OSMWithTags addTag(String key, String value) {
+    if (key == null || value == null) {
+      return this;
+    }
 
-    if (tags == null) tags = new HashMap<>();
+    if (tags == null) {
+      tags = new HashMap<>();
+    }
 
     tags.put(key.toLowerCase(), value);
+    return this;
   }
 
   /**

diff --git a/src/main/java/org/opentripplanner/openstreetmap/tagmapping/DefaultMapper.java b/src/main/java/org/opentripplanner/openstreetmap/tagmapping/DefaultMapper.java
@@ -1,6 +1,7 @@
 package org.opentripplanner.openstreetmap.tagmapping;
 
 import static org.opentripplanner.openstreetmap.wayproperty.MixinPropertiesBuilder.ofBicycleSafety;
+import static org.opentripplanner.openstreetmap.wayproperty.MixinPropertiesBuilder.ofWalkSafety;
 import static org.opentripplanner.openstreetmap.wayproperty.WayPropertiesBuilder.withModes;
 import static org.opentripplanner.street.model.StreetTraversalPermission.ALL;
 import static org.opentripplanner.street.model.StreetTraversalPermission.BICYCLE_AND_CAR;
@@ -618,6 +619,9 @@ public void populateProperties(WayPropertySet props) {
     props.setMixinProperties("CCGIS:bicycle:right=caution_area", ofBicycleSafety(1.45, 1));
     props.setMixinProperties("CCGIS:bicycle:left=caution_area", ofBicycleSafety(1, 1.45));
 
+    props.setMixinProperties("foot=discouraged", ofWalkSafety(3));
+    props.setMixinProperties("bicycle=discouraged", ofBicycleSafety(3));
+
     populateNotesAndNames(props);
 
     // slope overrides

diff --git a/src/main/java/org/opentripplanner/standalone/config/BuildConfig.java b/src/main/java/org/opentripplanner/standalone/config/BuildConfig.java
@@ -148,8 +148,6 @@ public class BuildConfig implements OtpDataStoreConfig {
   /** See {@link IslandPruningConfig}. */
   public final IslandPruningConfig islandPruning;
 
-  public final boolean banDiscouragedWalking;
-  public final boolean banDiscouragedBiking;
   public final Duration maxTransferDuration;
   public final NetexFeedParameters netexDefaults;
   public final GtfsFeedParameters gtfsDefaults;
@@ -211,18 +209,6 @@ public BuildConfig(NodeAdapter root, boolean logUnusedParams) {
             """
         )
         .asBoolean(false);
-    banDiscouragedWalking =
-      root
-        .of("banDiscouragedWalking")
-        .since(V2_0)
-        .summary("Should walking be allowed on OSM ways tagged with `foot=discouraged`")
-        .asBoolean(false);
-    banDiscouragedBiking =
-      root
-        .of("banDiscouragedBiking")
-        .since(V2_0)
-        .summary("Should biking be allowed on OSM ways tagged with `bicycle=discouraged`")
-        .asBoolean(false);
     configVersion =
       root
         .of("configVersion")

diff --git a/src/test/java/org/opentripplanner/openstreetmap/tagmapping/DefaultMapperTest.java b/src/test/java/org/opentripplanner/openstreetmap/tagmapping/DefaultMapperTest.java
@@ -2,8 +2,11 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.opentripplanner.street.model.StreetTraversalPermission.ALL;
 import static org.opentripplanner.street.model.StreetTraversalPermission.PEDESTRIAN;
+import static org.opentripplanner.street.model.StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE;
 
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.opentripplanner.openstreetmap.model.OSMWithTags;
 import org.opentripplanner.openstreetmap.wayproperty.SpeedPicker;
@@ -13,12 +16,15 @@
 
 public class DefaultMapperTest {
 
-  static WayPropertySet wps = new WayPropertySet();
+  private WayPropertySet wps;
   float epsilon = 0.01f;
 
-  static {
+  @BeforeEach
+  public void setup() {
+    var wps = new WayPropertySet();
     DefaultMapper source = new DefaultMapper();
     source.populateProperties(wps);
+    this.wps = wps;
   }
 
   /**
@@ -111,6 +117,32 @@ void stairs() {
     assertEquals(PEDESTRIAN, props.getPermission());
   }
 
+  @Test
+  void footDiscouraged() {
+    var regular = WayTestData.pedestrianTunnel();
+    var props = wps.getDataForWay(regular);
+    assertEquals(PEDESTRIAN_AND_BICYCLE, props.getPermission());
+    assertEquals(1, props.getWalkSafetyFeatures().forward());
+
+    var discouraged = WayTestData.pedestrianTunnel().addTag("foot", "discouraged");
+    var discouragedProps = wps.getDataForWay(discouraged);
+    assertEquals(PEDESTRIAN_AND_BICYCLE, discouragedProps.getPermission());
+    assertEquals(3, discouragedProps.getWalkSafetyFeatures().forward());
+  }
+
+  @Test
+  void bicycleDiscouraged() {
+    var regular = WayTestData.southeastLaBonitaWay();
+    var props = wps.getDataForWay(regular);
+    assertEquals(ALL, props.getPermission());
+    assertEquals(.98, props.getBicycleSafetyFeatures().forward());
+
+    var discouraged = WayTestData.southeastLaBonitaWay().addTag("bicycle", "discouraged");
+    var discouragedProps = wps.getDataForWay(discouraged);
+    assertEquals(ALL, discouragedProps.getPermission());
+    assertEquals(2.94, discouragedProps.getBicycleSafetyFeatures().forward(), epsilon);
+  }
+
   /**
    * Test that two values are within epsilon of each other.
    */

diff --git a/test/performance/norway/build-config.json b/test/performance/norway/build-config.json
@@ -14,8 +14,6 @@
     "islandWithoutStopsMaxSize": 5,
     "islandWithStopsMaxSize": 5
   },
-  "banDiscouragedWalking": false,
-  "banDiscouragedBiking": false,
   "distanceBetweenElevationSamples": 25,
   "multiThreadElevationCalculations": true,
   "boardingLocationTags": [],