Merge pull request #930 from conveyal/path-route-names

Include names in paths in CSV

src/main/java/com/conveyal/analysis/models/AnalysisRequest.java

@@ -21,6 +21,7 @@
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.List;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 /**
@@ -175,6 +176,14 @@ public class AnalysisRequest {
      */
     public int dualAccessibilityThreshold = 0;
 
+    /**
+     * Freeform (untyped) flags for enabling experimental, undocumented, or arcane behavior in backend or workers.
+     * This should be used to replace all previous special behavior flags that were embedded inside analysis names etc.
+     */
+    public Set<String> flags;
+
+    /** Control the details of CSV regional analysis output, including whether to output IDs, names, or both. */
+    public CsvResultOptions csvResultOptions = new CsvResultOptions();
 
     /**
      * Create the R5 `Scenario` from this request.
@@ -281,6 +290,8 @@ public void populateTask (AnalysisWorkerTask task, UserPermissions userPermissio
 
         task.includeTemporalDensity = includeTemporalDensity;
         task.dualAccessibilityThreshold = dualAccessibilityThreshold;
+        task.flags = flags;
+        task.csvResultOptions = csvResultOptions;
     }
 
     private EnumSet<LegMode> getEnumSetFromString (String s) {

src/main/java/com/conveyal/analysis/models/CsvResultOptions.java

@@ -0,0 +1,17 @@
+package com.conveyal.analysis.models;
+
+import com.conveyal.r5.transit.TransitLayer.EntityRepresentation;
+
+import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_ONLY;
+
+/**
+ * API model type included in analysis requests to control details of CSV regional analysis output.
+ * This type is shared between AnalysisRequest (Frontend -> Broker) and AnalysisWorkerTask (Broker -> Workers).
+ * There is precedent for nested compound types shared across those top level request types (see DecayFunction).
+ */
+public class CsvResultOptions {
+    public EntityRepresentation routeRepresentation = ID_ONLY;
+    public EntityRepresentation stopRepresentation = ID_ONLY;
+    // Only feed ID representation is allowed to be null (no feed IDs at all, the default).
+    public EntityRepresentation feedRepresentation = null;
+}

src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java

@@ -1,5 +1,6 @@
 package com.conveyal.r5.analyst.cluster;
 
+import com.conveyal.analysis.models.CsvResultOptions;
 import com.conveyal.r5.analyst.FreeFormPointSet;
 import com.conveyal.r5.analyst.Grid;
 import com.conveyal.r5.analyst.GridTransformWrapper;
@@ -15,6 +16,7 @@
 import com.fasterxml.jackson.annotation.JsonTypeInfo;
 
 import java.util.Arrays;
+import java.util.Set;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -177,6 +179,15 @@ public abstract class AnalysisWorkerTask extends ProfileRequest {
      */
     public ChaosParameters injectFault;
 
+    /**
+     * Freeform (untyped) flags for enabling experimental, undocumented, or arcane worker behavior.
+     * This should be used to replace all previous special behavior flags that were embedded inside analysis names etc.
+     */
+    public Set<String> flags;
+
+    /** Control the details of CSV regional analysis output, including whether to output IDs, names, or both. */
+    public CsvResultOptions csvResultOptions;
+
     /**
      * Is this a single point or regional request? Needed to encode types in JSON serialization. Can that type field be
      * added automatically with a serializer annotation instead of by defining a getter method and two dummy methods?

src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java

@@ -1,5 +1,6 @@
 package com.conveyal.r5.analyst.cluster;
 
+import com.conveyal.analysis.models.CsvResultOptions;
 import com.conveyal.r5.analyst.StreetTimesAndModes;
 import com.conveyal.r5.transit.TransitLayer;
 import com.conveyal.r5.transit.path.Path;
@@ -47,8 +48,11 @@ public class PathResult {
      * With additional changes, patterns could be collapsed further to route combinations or modes.
      */
     public final Multimap<RouteSequence, Iteration>[] iterationsForPathTemplates;
+
     private final TransitLayer transitLayer;
 
+    private final CsvResultOptions csvOptions;
+
     public static final String[] DATA_COLUMNS = new String[]{
             "routes",
             "boardStops",
@@ -76,6 +80,7 @@ public PathResult(AnalysisWorkerTask task, TransitLayer transitLayer) {
         }
         iterationsForPathTemplates = new Multimap[nDestinations];
         this.transitLayer = transitLayer;
+        this.csvOptions = task.csvResultOptions;
     }
 
     /**
@@ -108,7 +113,7 @@ public ArrayList<String[]>[] summarizeIterations(Stat stat) {
                     int nIterations = iterations.size();
                     checkState(nIterations > 0, "A path was stored without any iterations");
                     String waits = null, transfer = null, totalTime = null;
-                    String[] path = routeSequence.detailsWithGtfsIds(transitLayer);
+                    String[] path = routeSequence.detailsWithGtfsIds(transitLayer, csvOptions);
                     double targetValue;
                     IntStream totalWaits = iterations.stream().mapToInt(i -> i.waitTimes.sum());
                     if (stat == Stat.MINIMUM) {

src/main/java/com/conveyal/r5/transit/TransitLayer.java

@@ -54,6 +54,9 @@
 import java.util.stream.IntStream;
 import java.util.stream.StreamSupport;
 
+import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_ONLY;
+import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.NAME_ONLY;
+
 
 /**
  * A key simplifying factor is that we don't handle overnight trips. This is fine for analysis at usual times of day.
@@ -815,31 +818,59 @@ public TIntSet findStopsInGeometry (Geometry geometry) {
         return stops;
     }
 
+    public enum EntityRepresentation {
+        ID_ONLY, NAME_ONLY, NAME_AND_ID
+    }
+
     /**
      * For the given pattern index, returns the GTFS routeId. If includeName is true, the returned string will
      * also include a route_short_name or route_long_name (if they are not null).
      */
-    public String routeString(int routeIndex, boolean includeName) {
+    public String routeString (int routeIndex, EntityRepresentation nameOrId) {
         RouteInfo routeInfo = routes.get(routeIndex);
-        String route = routeInfo.route_id;
-        if (includeName) {
-            if (routeInfo.route_short_name != null) {
-                route += " (" + routeInfo.route_short_name + ")";
-            } else if (routeInfo.route_long_name != null){
-                route += " (" + routeInfo.route_long_name + ")";
+        String name = routeInfo.route_short_name;
+        String id = routeInfo.route_id;
+        // If we might actually use the name, check some fallbacks.
+        if (nameOrId != ID_ONLY) {
+            if (name == null) {
+                name = routeInfo.route_long_name;
+            }
+            if (name == null) {
+                name = routeInfo.route_id;
             }
         }
-        return route;
+        return switch (nameOrId) {
+            case NAME_ONLY -> name;
+            case NAME_AND_ID -> name + " (" + id + ")";
+            default -> id;
+        };
     }
 
     /**
      * For the given stop index, returns the GTFS stopId (stripped of R5's feedId prefix) and, if includeName is true,
      * stopName.
      */
-    public String stopString(int stopIndex, boolean includeName) {
-        // TODO use a compact feed index, instead of splitting to remove feedIds
-        String stop = stopIdForIndex.get(stopIndex) == null ? "[new]" : stopIdForIndex.get(stopIndex).split(":")[1];
-        if (includeName) stop += " (" + stopNames.get(stopIndex) + ")";
-        return stop;
+    public String stopString(int stopIndex, EntityRepresentation nameOrId) {
+        String stopId = stopIdForIndex.get(stopIndex);
+        String stopName = stopNames.get(stopIndex);
+        // I'd trust the JVM JIT to optimize out these assignments on different code paths, but not the split call.
+        if (nameOrId != NAME_ONLY) {
+            if (stopId == null) {
+                stopId = "[new]";
+            } else {
+                // TODO use a compact feed ID instead of splitting to remove feedIds (or put feedId into another CSV field)
+                stopId = stopId.split(":")[1];
+            }
+        }
+        if (nameOrId != ID_ONLY) {
+            if (stopName == null) {
+                stopName = "[new]";
+            }
+        }
+        return switch (nameOrId) {
+            case NAME_ONLY -> stopName;
+            case NAME_AND_ID -> stopName + " (" + stopId + ")";
+            default -> stopId;
+        };
     }
 }

src/main/java/com/conveyal/r5/transit/path/RouteSequence.java

@@ -1,6 +1,8 @@
 package com.conveyal.r5.transit.path;
 
+import com.conveyal.analysis.models.CsvResultOptions;
 import com.conveyal.r5.transit.TransitLayer;
+import com.conveyal.r5.transit.TransitLayer.EntityRepresentation;
 import gnu.trove.list.TIntList;
 import gnu.trove.list.array.TIntArrayList;
 
@@ -9,6 +11,8 @@
 import java.util.Objects;
 import java.util.StringJoiner;
 
+import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.NAME_AND_ID;
+
 /** A door-to-door path that includes the routes ridden between stops */
 public class RouteSequence {
 
@@ -28,15 +32,15 @@ public RouteSequence(PatternSequence patternSequence, TransitLayer transitLayer)
     }
 
     /** Returns details summarizing this route sequence, using GTFS ids stored in the supplied transitLayer. */
-    public String[] detailsWithGtfsIds(TransitLayer transitLayer){
+    public String[] detailsWithGtfsIds (TransitLayer transitLayer, CsvResultOptions csvOptions){
         StringJoiner routeIds = new StringJoiner("|");
         StringJoiner boardStopIds = new StringJoiner("|");
         StringJoiner alightStopIds = new StringJoiner("|");
         StringJoiner rideTimes = new StringJoiner("|");
         for (int i = 0; i < routes.size(); i++) {
-            routeIds.add(transitLayer.routeString(routes.get(i), false));
-            boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), false));
-            alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), false));
+            routeIds.add(transitLayer.routeString(routes.get(i), csvOptions.routeRepresentation));
+            boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), csvOptions.stopRepresentation));
+            alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), csvOptions.stopRepresentation));
             rideTimes.add(String.format("%.1f", stopSequence.rideTimesSeconds.get(i) / 60f));
         }
         String accessTime = stopSequence.access == null ? null : String.format("%.1f", stopSequence.access.time / 60f);
@@ -55,9 +59,9 @@ public String[] detailsWithGtfsIds(TransitLayer transitLayer){
     public Collection<TransitLeg> transitLegs(TransitLayer transitLayer) {
         Collection<TransitLeg> transitLegs = new ArrayList<>();
         for (int i = 0; i < routes.size(); i++) {
-            String routeString = transitLayer.routeString(routes.get(i), true);
-            String boardStop = transitLayer.stopString(stopSequence.boardStops.get(i), true);
-            String alightStop = transitLayer.stopString(stopSequence.alightStops.get(i), true);
+            String routeString = transitLayer.routeString(routes.get(i), NAME_AND_ID);
+            String boardStop = transitLayer.stopString(stopSequence.boardStops.get(i), NAME_AND_ID);
+            String alightStop = transitLayer.stopString(stopSequence.alightStops.get(i), NAME_AND_ID);
             transitLegs.add(new TransitLeg(routeString, stopSequence.rideTimesSeconds.get(i), boardStop, alightStop));
         }
         return transitLegs;