From e5acb4312464fecccd52659b1dbeecd0886386d5 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Fri, 17 Mar 2023 21:22:37 +0800 Subject: [PATCH 1/2] make all row numbers one-based for consistency Some error messages (particularly those produced in post-validation) had row numbers based on entity.sourceFileLine which is one-based, while others used raw zero-based row numbers. --- .../java/com/conveyal/gtfs/model/Agency.java | 2 +- .../java/com/conveyal/gtfs/model/Calendar.java | 2 +- .../com/conveyal/gtfs/model/CalendarDate.java | 2 +- .../java/com/conveyal/gtfs/model/Entity.java | 16 ++++++++++++---- .../com/conveyal/gtfs/model/FareAttribute.java | 2 +- .../java/com/conveyal/gtfs/model/FareRule.java | 2 +- .../java/com/conveyal/gtfs/model/FeedInfo.java | 2 +- .../java/com/conveyal/gtfs/model/Frequency.java | 2 +- src/main/java/com/conveyal/gtfs/model/Route.java | 2 +- .../java/com/conveyal/gtfs/model/ShapePoint.java | 2 +- src/main/java/com/conveyal/gtfs/model/Stop.java | 2 +- .../java/com/conveyal/gtfs/model/StopTime.java | 2 +- .../java/com/conveyal/gtfs/model/Transfer.java | 3 ++- src/main/java/com/conveyal/gtfs/model/Trip.java | 2 +- 14 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/model/Agency.java b/src/main/java/com/conveyal/gtfs/model/Agency.java index a2e301329..ce03407af 100644 --- a/src/main/java/com/conveyal/gtfs/model/Agency.java +++ b/src/main/java/com/conveyal/gtfs/model/Agency.java @@ -34,7 +34,7 @@ protected boolean isRequired() { @Override public void loadOneRow() throws IOException { Agency a = new Agency(); - a.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + a.sourceFileLine = row; a.agency_id = getStringField("agency_id", false); // can only be absent if there is a single agency -- requires a special validator. a.agency_name = getStringField("agency_name", true); a.agency_url = getUrlField("agency_url", true); diff --git a/src/main/java/com/conveyal/gtfs/model/Calendar.java b/src/main/java/com/conveyal/gtfs/model/Calendar.java index e0f0be87f..e40213cb5 100644 --- a/src/main/java/com/conveyal/gtfs/model/Calendar.java +++ b/src/main/java/com/conveyal/gtfs/model/Calendar.java @@ -55,7 +55,7 @@ public void loadOneRow() throws IOException { feed.errors.add(new DuplicateKeyError(tableName, row, "service_id")); } else { Calendar c = new Calendar(); - c.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + c.sourceFileLine = row; c.service_id = service.service_id; c.monday = getIntField("monday", true, 0, 1); c.tuesday = getIntField("tuesday", true, 0, 1); diff --git a/src/main/java/com/conveyal/gtfs/model/CalendarDate.java b/src/main/java/com/conveyal/gtfs/model/CalendarDate.java index 081f4c8b3..eb8f46902 100644 --- a/src/main/java/com/conveyal/gtfs/model/CalendarDate.java +++ b/src/main/java/com/conveyal/gtfs/model/CalendarDate.java @@ -55,7 +55,7 @@ public void loadOneRow() throws IOException { feed.errors.add(new DuplicateKeyError(tableName, row, "(service_id, date)")); } else { CalendarDate cd = new CalendarDate(); - cd.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + cd.sourceFileLine = row; cd.service_id = service_id; cd.date = date; cd.exception_type = getIntField("exception_type", true, 1, 2); diff --git a/src/main/java/com/conveyal/gtfs/model/Entity.java b/src/main/java/com/conveyal/gtfs/model/Entity.java index c445f2a41..e457f5662 100644 --- a/src/main/java/com/conveyal/gtfs/model/Entity.java +++ b/src/main/java/com/conveyal/gtfs/model/Entity.java @@ -81,7 +81,11 @@ public static abstract class Loader { protected final Set missingRequiredColumns = Sets.newHashSet(); protected CsvReader reader; - protected int row; + + // Use one-based rows which are easier to understand in error messages. + // Line numbers are conventionally one-based even in IDEs and programming editors. + protected int row = 1; + // TODO "String column" that is set before any calls to avoid passing around the column name public Loader(GTFSFeed feed, String tableName) { @@ -285,18 +289,22 @@ public void loadTable (ZipFile zip) throws IOException { } CsvReader reader = new CsvReader(inStream, ',', Charset.forName("UTF8")); this.reader = reader; + // row number is already positioned at 1 boolean hasHeaders = reader.readHeaders(); if (!hasHeaders) { feed.errors.add(new EmptyTableError(tableName)); } while (reader.readRecord()) { - // reader.getCurrentRecord() is zero-based and does not include the header line, keep our own row count - if (++row % 500000 == 0) { + // reader.getCurrentRecord() is zero-based and does not include the header line, + // so we keep our own one-based row count that does include the header line. + // Advance row number before calling loadOneRow so the current value is available inside that function. + row += 1; + if (row % 500000 == 0) { LOG.info("Record number {}", human(row)); } loadOneRow(); // Call subclass method to produce an entity from the current row. } - if (row == 0) { + if (row < 2) { feed.errors.add(new EmptyTableError(tableName)); } } diff --git a/src/main/java/com/conveyal/gtfs/model/FareAttribute.java b/src/main/java/com/conveyal/gtfs/model/FareAttribute.java index 35afcbc97..69e15ff81 100644 --- a/src/main/java/com/conveyal/gtfs/model/FareAttribute.java +++ b/src/main/java/com/conveyal/gtfs/model/FareAttribute.java @@ -42,7 +42,7 @@ public void loadOneRow() throws IOException { feed.errors.add(new DuplicateKeyError(tableName, row, "fare_id")); } else { FareAttribute fa = new FareAttribute(); - fa.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + fa.sourceFileLine = row; fa.fare_id = fareId; fa.price = getDoubleField("price", true, 0, Integer.MAX_VALUE); fa.currency_type = getStringField("currency_type", true); diff --git a/src/main/java/com/conveyal/gtfs/model/FareRule.java b/src/main/java/com/conveyal/gtfs/model/FareRule.java index ce8b3c033..04cfdb7bb 100644 --- a/src/main/java/com/conveyal/gtfs/model/FareRule.java +++ b/src/main/java/com/conveyal/gtfs/model/FareRule.java @@ -43,7 +43,7 @@ public void loadOneRow() throws IOException { Fare fare = fares.computeIfAbsent(fareId, Fare::new); FareRule fr = new FareRule(); - fr.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + fr.sourceFileLine = row; fr.fare_id = fare.fare_id; fr.route_id = getStringField("route_id", false); fr.origin_id = getStringField("origin_id", false); diff --git a/src/main/java/com/conveyal/gtfs/model/FeedInfo.java b/src/main/java/com/conveyal/gtfs/model/FeedInfo.java index cfedca049..041a2f943 100644 --- a/src/main/java/com/conveyal/gtfs/model/FeedInfo.java +++ b/src/main/java/com/conveyal/gtfs/model/FeedInfo.java @@ -41,7 +41,7 @@ protected boolean isRequired() { @Override public void loadOneRow() throws IOException { FeedInfo fi = new FeedInfo(); - fi.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + fi.sourceFileLine = row; fi.feed_id = getStringField("feed_id", false); fi.feed_publisher_name = getStringField("feed_publisher_name", true); fi.feed_publisher_url = getUrlField("feed_publisher_url", true); diff --git a/src/main/java/com/conveyal/gtfs/model/Frequency.java b/src/main/java/com/conveyal/gtfs/model/Frequency.java index e327224aa..558d7bc41 100644 --- a/src/main/java/com/conveyal/gtfs/model/Frequency.java +++ b/src/main/java/com/conveyal/gtfs/model/Frequency.java @@ -57,7 +57,7 @@ protected boolean isRequired() { public void loadOneRow() throws IOException { Frequency f = new Frequency(); Trip trip = getRefField("trip_id", true, feed.trips); - f.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + f.sourceFileLine = row; f.trip_id = trip.trip_id; f.start_time = getTimeField("start_time", true); f.end_time = getTimeField("end_time", true); diff --git a/src/main/java/com/conveyal/gtfs/model/Route.java b/src/main/java/com/conveyal/gtfs/model/Route.java index cb9c208b8..be5a98c77 100644 --- a/src/main/java/com/conveyal/gtfs/model/Route.java +++ b/src/main/java/com/conveyal/gtfs/model/Route.java @@ -46,7 +46,7 @@ protected boolean isRequired() { @Override public void loadOneRow() throws IOException { Route r = new Route(); - r.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + r.sourceFileLine = row; r.route_id = getStringField("route_id", true); Agency agency = getRefField("agency_id", false, feed.agency); diff --git a/src/main/java/com/conveyal/gtfs/model/ShapePoint.java b/src/main/java/com/conveyal/gtfs/model/ShapePoint.java index 904cb939b..e5a7e71d1 100644 --- a/src/main/java/com/conveyal/gtfs/model/ShapePoint.java +++ b/src/main/java/com/conveyal/gtfs/model/ShapePoint.java @@ -46,7 +46,7 @@ public void loadOneRow() throws IOException { double shape_dist_traveled = getDoubleField("shape_dist_traveled", false, 0D, Double.MAX_VALUE); ShapePoint s = new ShapePoint(shape_id, shape_pt_lat, shape_pt_lon, shape_pt_sequence, shape_dist_traveled); - s.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + s.sourceFileLine = row; feed.shape_points.put(new Tuple2(s.shape_id, s.shape_pt_sequence), s); } } diff --git a/src/main/java/com/conveyal/gtfs/model/Stop.java b/src/main/java/com/conveyal/gtfs/model/Stop.java index 580030bef..e121a71bd 100644 --- a/src/main/java/com/conveyal/gtfs/model/Stop.java +++ b/src/main/java/com/conveyal/gtfs/model/Stop.java @@ -43,7 +43,7 @@ protected boolean isRequired() { @Override public void loadOneRow() throws IOException { Stop s = new Stop(); - s.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + s.sourceFileLine = row; s.stop_id = getStringField("stop_id", true); s.stop_code = getStringField("stop_code", false); s.stop_name = getStringField("stop_name", true); diff --git a/src/main/java/com/conveyal/gtfs/model/StopTime.java b/src/main/java/com/conveyal/gtfs/model/StopTime.java index b15f7b46d..7a88d217e 100644 --- a/src/main/java/com/conveyal/gtfs/model/StopTime.java +++ b/src/main/java/com/conveyal/gtfs/model/StopTime.java @@ -50,7 +50,7 @@ protected boolean isRequired() { @Override public void loadOneRow() throws IOException { StopTime st = new StopTime(); - st.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + st.sourceFileLine = row; st.trip_id = getStringField("trip_id", true); // TODO: arrival_time and departure time are not required, but if one is present the other should be // also, if this is the first or last stop, they are both required diff --git a/src/main/java/com/conveyal/gtfs/model/Transfer.java b/src/main/java/com/conveyal/gtfs/model/Transfer.java index 56066c373..3af7b970b 100644 --- a/src/main/java/com/conveyal/gtfs/model/Transfer.java +++ b/src/main/java/com/conveyal/gtfs/model/Transfer.java @@ -31,7 +31,7 @@ protected boolean isRequired() { @Override public void loadOneRow() throws IOException { Transfer tr = new Transfer(); - tr.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + tr.sourceFileLine = row; tr.from_stop_id = getStringField("from_stop_id", true); tr.to_stop_id = getStringField("to_stop_id", true); tr.transfer_type = getIntField("transfer_type", true, 0, 3); @@ -48,6 +48,7 @@ public void loadOneRow() throws IOException { getRefField("from_trip_id", false, feed.trips); getRefField("to_trip_id", false, feed.trips); + // row number used as an arbitrary unique string to give MapDB a key. feed.transfers.put(Long.toString(row), tr); } diff --git a/src/main/java/com/conveyal/gtfs/model/Trip.java b/src/main/java/com/conveyal/gtfs/model/Trip.java index 601d4d60d..087f11b76 100644 --- a/src/main/java/com/conveyal/gtfs/model/Trip.java +++ b/src/main/java/com/conveyal/gtfs/model/Trip.java @@ -35,7 +35,7 @@ protected boolean isRequired() { public void loadOneRow() throws IOException { Trip t = new Trip(); - t.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index + t.sourceFileLine = row; t.route_id = getStringField("route_id", true); t.service_id = getStringField("service_id", true); t.trip_id = getStringField("trip_id", true); From 02886ea1231fc9688eb86a40e23f99c26ca2cb80 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Sat, 18 Mar 2023 01:25:33 +0800 Subject: [PATCH 2/2] get polyline encoder 0.2 from Conveyal Maven repo --- build.gradle | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index 3ab0dbb0c..618a7bb88 100644 --- a/build.gradle +++ b/build.gradle @@ -125,10 +125,8 @@ repositories { // Put Open Source Geospatial before Maven Central to get JAI core, see https://stackoverflow.com/a/26993223 maven { url 'https://repo.osgeo.org/repository/release/' } mavenCentral() - // TODO review whether we really need the repositories below + // Polyline encoder 0.2 is now in Maven repo maven { url 'https://maven.conveyal.com' } - // For the polyline encoder - maven { url 'https://nexus.axiomalaska.com/nexus/content/repositories/public-releases' } } // Exclude all JUnit 4 transitive dependencies - IntelliJ bug causes it to think we're using Junit 4 instead of 5.