From 648fddca9d84d62ec89c8c562b1cb7cf924f8b62 Mon Sep 17 00:00:00 2001 From: rafapereirabr Date: Sun, 1 Nov 2026 00:18:37 -0300 Subject: [PATCH 1/6] set_dwell_time() gains strategy argument (push_trip_end, keep_total_duration) Co-Authored-By: Claude Opus 5.5 --- NEWS.md | 5 + R/set_dwell_time.R | 153 ++++++++++++---- man/set_dwell_time.Rd | 56 +++++- tests/testthat/test-set_dwell_time.R | 263 +++++++++++++++++++++++++++ 4 files changed, 439 insertions(+), 38 deletions(-) diff --git a/NEWS.md b/NEWS.md index fc55d4e3..5d1f099b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,7 +1,12 @@ # gtfstools (development version) +## Breaking changes + +- In `set_dwell_time()`, the arguments after `unit` must now be passed by name, since the new `strategy` argument comes right after it. + ## New features +- `set_dwell_time()` gains `strategy`. The default, `"push_trip_end"`, shifts the later times of the trips, as before. `"keep_total_duration"` splits the change around each intermediate stop, keeping the trip durations. - `interpolate_stop_times()` gains `overwrite` and `keep_dwell_times`. With `overwrite = TRUE`, the times of every stop between the first and the last stop of each trip are replaced by interpolated ones, instead of only the blank ones. By default (`keep_dwell_times = TRUE`), these stops keep their original dwell times. ## Bug fixes diff --git a/R/set_dwell_time.R b/R/set_dwell_time.R index f4a2d0f4..a1ba472e 100644 --- a/R/set_dwell_time.R +++ b/R/set_dwell_time.R @@ -1,9 +1,11 @@ #' Set dwell time #' #' Sets the dwell time of each specified `trip_id` at each specified `stop_id`, -#' i.e. the time the vehicle stays at the stop, by changing the -#' `departure_time` at these stops in `stop_times` and shifting the later times -#' of the trips accordingly. +#' i.e. the time the vehicle stays at the stop for passengers to board and +#' alight, by changing the times at these stops in `stop_times`. By default, +#' the later times of the trips are shifted accordingly. Alternatively, the +#' trip durations are kept and the running times between stops absorb the +#' change. #' #' @template gtfs #' @param dwell_time A non-negative number, the dwell time to be set, in @@ -13,6 +15,20 @@ #' @param unit A string representing the time unit in which `dwell_time` is #' given. One of `"s"` (seconds, the default), `"min"` (minutes), `"h"` #' (hours) or `"d"` (days). +#' @param strategy A string, how the trips absorb the changes in dwell time. +#' One of: +#' - `"shift_trip_end"` (the default): keeps the running times between +#' stops and shifts all the later times of the trip, so the arrival at the +#' last stop moves later (or earlier, if dwell times are shortened). It +#' preserves the implicit average speed between stops but changes the +#' trip duration. +#' - `"keep_total_duration"`: centres each intermediate stop's dwell on its +#' original time, taking half of the change from the running time before +#' the stop and half from the one after it. The first and last stops are +#' not changed, so the trip keeps its duration, its arrival at the last +#' stop and the implicit average speed between its first and last stops. +#' An error is raised if a running time would become negative. +#' #' @param trip_id A character vector including the `trip_id`s to have their #' dwell times set. If `NULL` (the default), the dwell times of every #' `trip_id` in the GTFS (or of every trip of the given `route_type`s) are @@ -35,25 +51,10 @@ #' invisibly (note that in this case the original GTFS object is altered). #' #' @section Details: -#' The dwell time of a visit is its `departure_time` minus its `arrival_time` -#' in `stop_times`, as returned by [get_dwell_time()]. It is set at the visits -#' of every specified trip to every specified stop (not of pairs formed by -#' their elements): the arrival time is kept and the departure time is set to -#' the arrival time plus `dwell_time`. All the later times of the trip, in -#' `stop_sequence` order, are shifted by the change, and changes at several -#' visits of the same trip add up. At a trip's last stop, only the departure -#' time changes. A stop visited more than once by the same trip has its dwell -#' time set at each visit. #' -#' Blank times are never filled. Selected visits with a blank arrival or -#' departure time are not changed and don't shift the later times, with a -#' warning when `stop_id` is given; fill them beforehand with -#' [interpolate_stop_times()]. An error is raised if a shift would result in a -#' negative time (times not in chronological order) or in a time later than -#' `"9999:59:59"`. `stop_sequence` is assumed to be unique within each trip -#' and not `NA`. When both `trip_id` and `stop_id` are given but none of the -#' trips visits any of the stops, a warning is raised and the GTFS is returned -#' unchanged. +#' Since the first stop is never changed, the departures of trips listed in +#' `frequencies` are not changed either. Blank arrival or departure times +#' are never filled. #' #' For trips listed in `frequencies`, the times in `stop_times` are a template #' (see [frequencies_to_stop_times()]), so setting the dwell time at the first @@ -118,6 +119,18 @@ #' ) #' get_dwell_time(new_gtfs, trip_id = "CPTM L07-0") #' +#' # keep the trip duration: the running times between the stops absorb the +#' # dwell times, and the arrival at the last stop doesn't change +#' new_gtfs <- set_dwell_time( +#' gtfs, +#' trip_id = "CPTM L07-0", +#' dwell_time = 30, +#' strategy = "keep_total_duration" +#' ) +#' new_gtfs$stop_times |> +#' filter(trip_id == "CPTM L07-0") |> +#' tail() +#' #' # original gtfs remains unchanged #' get_dwell_time(gtfs, stop_id = "18960") #' @@ -139,6 +152,7 @@ set_dwell_time <- function( gtfs, dwell_time, unit = "s", + strategy = "shift_trip_end", trip_id = NULL, stop_id = NULL, from = NULL, @@ -151,6 +165,11 @@ set_dwell_time <- function( checkmate::assert_character(stop_id, null.ok = TRUE, any.missing = FALSE) checkmate::assert_string(unit) checkmate::assert_names(unit, subset.of = names(dwell_unit_factors)) + checkmate::assert_string(strategy) + checkmate::assert_names( + strategy, + subset.of = c("shift_trip_end", "keep_total_duration") + ) checkmate::assert_number( dwell_time, lower = 0, @@ -184,7 +203,8 @@ set_dwell_time <- function( stop_id, window, route_type, - by_reference + by_reference, + strategy = strategy ) if (by_reference) { @@ -195,11 +215,12 @@ set_dwell_time <- function( } - # sets the dwell time of the selected visits to 'dwell_secs' and shifts the # later times of their trips, as described in set_dwell_time(). with # 'raise_only', only shorter dwell times are raised, as described in -# set_min_dwell_time(). the arguments are validated by these functions +# set_min_dwell_time(). with strategy = "keep_total_duration", the change is +# split around each intermediate visit instead. the arguments are validated by +# these functions change_dwell_times <- function( gtfs, @@ -209,7 +230,8 @@ change_dwell_times <- function( window, route_type, by_reference, - raise_only = FALSE + raise_only = FALSE, + strategy = "shift_trip_end" ) { if (!is.null(route_type)) { type_routes <- route_type_ids(gtfs, route_type) @@ -303,11 +325,12 @@ change_dwell_times <- function( ) } - # a minimum skips the first and last visits with both times, and raises - # negative dwell times like any other shorter one. it comes after the - # blank warning, which needs the selection before it's narrowed + # a minimum and "keep_total_duration" skip the first and last visits with + # both times. a minimum also raises negative dwell times like any other + # shorter one. it comes after the blank warning, which needs the selection + # before it's narrowed - if (raise_only) { + if (raise_only || strategy == "keep_total_duration") { timed <- st[ !is.na(arr) & !is.na(dep) & !is.na(trip_id) & !is.na(stop_sequence), c("row", "trip_id", "stop_sequence") @@ -318,12 +341,15 @@ change_dwell_times <- function( st[, selected := selected & row %in% timed$row[is_middle] & - dep - arr < dwell_secs + (!raise_only | dep - arr < dwell_secs) ] } - # each selected call shifts its own departure and all the later times of - # its trip by the change in its dwell time + # with "shift_trip_end", each selected call shifts its own departure and + # all the later times of its trip by the change in its dwell time. with + # "keep_total_duration", the change is split around the call (an odd + # second goes to the departure), and the running times to and from it + # absorb it # the shifts are accumulated as doubles, so that a sum beyond the integer # range raises the error below instead of silently becoming NA @@ -331,12 +357,69 @@ change_dwell_times <- function( st[, delta := 0] st[selected & !is_blank, delta := as.numeric(arr) + dwell_secs - dep] data.table::setorderv(st, c("trip_id", "stop_sequence")) - st[, dep_shift := cumsum(delta), by = trip_id] - st[, arr_shift := dep_shift - delta] - st <- st[dep_shift != 0 | arr_shift != 0] + if (strategy == "shift_trip_end") { + st[, dep_shift := cumsum(delta), by = trip_id] + st[, arr_shift := dep_shift - delta] + } else { + st[, arr_shift := -trunc(delta / 2)] + st[, dep_shift := delta + arr_shift] + } + st[, `:=`(new_arr = arr + arr_shift, new_dep = dep + dep_shift)] + # with "keep_total_duration", a running time can't absorb more than it + # has. each one is measured from the previous timed call, skipping blank + # ones. running times that were already negative are left to the range + # check below + + # 'st' is ordered by trip, so the previous timed call is found without + # grouping, which would be slow with many trips. calls with no previous + # timed call in their trip get NA. new times are blank where old ones are + + if (strategy == "keep_total_duration") { + end <- data.table::fcoalesce(st$dep, st$arr) + new_end <- data.table::fcoalesce(st$new_dep, st$new_arr) + row_index <- seq_len(nrow(st)) + last_timed <- data.table::nafill( + data.table::fifelse(is.na(end), NA_integer_, row_index), + "locf" + ) + prev <- data.table::shift(last_timed) + is_trip_start <- !duplicated(st$trip_id) + trip_start <- row_index[is_trip_start][cumsum(is_trip_start)] + prev[prev < trip_start] <- NA + + is_negative <- which( + data.table::fcoalesce(st$arr, st$dep) - end[prev] >= 0 & + data.table::fcoalesce(st$new_arr, st$new_dep) - new_end[prev] < 0 + ) + + if (length(is_negative) > 0) { + negative_trips <- unique(st$trip_id[is_negative]) + abbreviated <- abbreviate_ids(negative_trips) + cli::cli_abort( + c( + paste0( + "Setting the dwell time with ", + "{.code strategy = \"keep_total_duration\"} results in ", + "negative running times between stops in the following ", + "{cli::qty(length(negative_trips))}trip{?s}: ", + "{.val {abbreviated$shown}}{abbreviated$more}." + ), + "i" = paste0( + "Use a shorter {.arg dwell_time}, select fewer stops or trips, ", + "or use {.code strategy = \"shift_trip_end\"}." + ) + ), + class = "gtfstools_negative_running_time", + call = parent.frame() + ) + } + } + + st <- st[dep_shift != 0 | arr_shift != 0] + # check before writing, so that by_reference doesn't leave a half-edited # table behind diff --git a/man/set_dwell_time.Rd b/man/set_dwell_time.Rd index 24ea5d30..a674aff8 100644 --- a/man/set_dwell_time.Rd +++ b/man/set_dwell_time.Rd @@ -8,6 +8,7 @@ set_dwell_time( gtfs, dwell_time, unit = "s", + strategy = "push_trip_end", trip_id = NULL, stop_id = NULL, from = NULL, @@ -28,6 +29,22 @@ times).} given. One of \code{"s"} (seconds, the default), \code{"min"} (minutes), \code{"h"} (hours) or \code{"d"} (days).} +\item{strategy}{A string, how the trips absorb the changes in dwell time. +One of: +\itemize{ +\item \code{"push_trip_end"} (the default): keeps the running times between +stops and shifts all the later times of the trip, so the arrival at the +last stop moves later (or earlier, if dwell times are shortened). It +preserves the implicit average speed between stops but changes the +trip duration. +\item \code{"keep_total_duration"}: centres each intermediate stop's dwell on its +original time, taking half of the change from the running time before +the stop and half from the one after it. The first and last stops are +not changed, so the trip keeps its duration, its arrival at the last +stop and the implicit average speed between its first and last stops. +An error is raised if a running time would become negative. +}} + \item{trip_id}{A character vector including the \code{trip_id}s to have their dwell times set. If \code{NULL} (the default), the dwell times of every \code{trip_id} in the GTFS (or of every trip of the given \code{route_type}s) are @@ -61,12 +78,18 @@ invisibly (note that in this case the original GTFS object is altered). } \description{ Sets the dwell time of each specified \code{trip_id} at each specified \code{stop_id}, -i.e. the time the vehicle stays at the stop, by changing the -\code{departure_time} at these stops in \code{stop_times} and shifting the later times -of the trips accordingly. +i.e. the time the vehicle stays at the stop for passengers to board and +alight, by changing the times at these stops in \code{stop_times}. By default, +the later times of the trips are shifted accordingly. Alternatively, the +trip durations are kept and the running times between stops absorb the +change. } \section{Details}{ +The following paragraphs describe the default, +\code{strategy = "push_trip_end"}. See the last paragraph for +\code{strategy = "keep_total_duration"}. + The dwell time of a visit is its \code{departure_time} minus its \code{arrival_time} in \code{stop_times}, as returned by \code{\link[=get_dwell_time]{get_dwell_time()}}. It is set at the visits of every specified trip to every specified stop (not of pairs formed by @@ -106,6 +129,21 @@ never stations: to set the dwell times at a station, pass its children that appear in \code{stop_times} (see \code{\link[=get_children_stops]{get_children_stops()}}). Existing \verb{_secs} columns in \code{stop_times} (e.g. created with \code{\link[=convert_time_to_seconds]{convert_time_to_seconds()}}) are updated to match. + +With \code{strategy = "keep_total_duration"}, visits are selected in the same +way, but the change in dwell time is split around each visit: when the +dwell time is lengthened, its arrival is moved earlier by half of the change +(rounded toward zero, so an odd second goes to the departure) and its +departure later by the rest, and the other way around when it's shortened. +No other time of the trip changes, so the running times to and from the +visit absorb the change (the vehicle runs faster when dwell times are +lengthened) and the trip keeps its duration. The first and last visits with +both an arrival and a departure time of each trip are never changed, as in +\code{\link[=set_min_dwell_time]{set_min_dwell_time()}}. An error is raised if a running time would become +negative, i.e. if the dwell times set take more time than the trip spends +running between the stops (running times that are already negative are not +checked). Since the first stop is never changed, the departures of trips +listed in \code{frequencies} are not changed either. } \examples{ @@ -150,6 +188,18 @@ new_gtfs <- set_dwell_time( ) get_dwell_time(new_gtfs, trip_id = "CPTM L07-0") +# keep the trip duration: the running times between the stops absorb the +# dwell times, and the arrival at the last stop doesn't change +new_gtfs <- set_dwell_time( + gtfs, + trip_id = "CPTM L07-0", + dwell_time = 30, + strategy = "keep_total_duration" +) +new_gtfs$stop_times |> + filter(trip_id == "CPTM L07-0") |> + tail() + # original gtfs remains unchanged get_dwell_time(gtfs, stop_id = "18960") diff --git a/tests/testthat/test-set_dwell_time.R b/tests/testthat/test-set_dwell_time.R index 8a5027e6..cb3a3011 100644 --- a/tests/testthat/test-set_dwell_time.R +++ b/tests/testthat/test-set_dwell_time.R @@ -11,6 +11,7 @@ tester <- function(gtfs = get("gtfs", envir = parent.frame()), unit = "s", from = NULL, to = NULL, + strategy = "push_trip_end", by_reference = FALSE) { set_dwell_time( gtfs, @@ -20,10 +21,32 @@ tester <- function(gtfs = get("gtfs", envir = parent.frame()), unit = unit, from = from, to = to, + strategy = strategy, by_reference = by_reference ) } +# builds a feed whose 'stop_times' has the given trips. each trip is a list +# with stop_ids and arrival and departure times + +speed_gtfs <- function(...) { + trips <- list(...) + new_gtfs <- read_gtfs(spo_path) + new_gtfs$stop_times <- data.table::rbindlist( + lapply(names(trips), function(trip) { + x <- trips[[trip]] + data.table::data.table( + trip_id = trip, + stop_id = x$stop_id, + stop_sequence = seq_along(x$stop_id), + arrival_time = x$arr, + departure_time = x$dep + ) + }) + ) + return(new_gtfs) +} + # small hand-built feed with a single trip, "t", whose rows are not ordered by # stop_sequence. it visits A twice (a loop), C with blank times, and runs past # midnight. its dwell times are 60 (A), 30 (B), NA (C), 0 (A) and 60 (D) @@ -83,6 +106,9 @@ test_that("raises errors due to incorrect input types/value", { expect_error(tester(by_reference = "TRUE")) expect_error(tester(by_reference = NA)) expect_error(tester(by_reference = c(TRUE, TRUE))) + expect_error(tester(strategy = "x")) + expect_error(tester(strategy = c("push_trip_end", "keep_total_duration"))) + expect_error(tester(strategy = NA)) # dwell_time is the second argument, so it can be given by position @@ -595,3 +621,240 @@ test_that("route_type restricts the changed visits", { expect_error(set_dwell_time(gtfs, dwell_time = 30, route_type = "2")) }) + +test_that("keep_total_duration splits the change and keeps the trip duration", { + + # t has no dwell times. u starts with a blank visit, which is skipped and + # doesn't carry t's times into u + + four_gtfs <- speed_gtfs( + t = list( + stop_id = c("A", "B", "C", "D"), + arr = c("08:00:00", "08:10:00", "08:20:00", "08:40:00"), + dep = c("08:00:00", "08:10:00", "08:20:00", "08:40:00") + ), + u = list( + stop_id = c("W", "X", "Y", "Z"), + arr = c("", "07:00:00", "07:10:00", "07:20:00"), + dep = c("", "07:00:00", "07:10:00", "07:20:00") + ) + ) + + new_gtfs <- tester( + four_gtfs, + dwell_time = 60, + strategy = "keep_total_duration" + ) + expect_identical( + trip_times(new_gtfs, "t"), + data.table::data.table( + stop_id = c("A", "B", "C", "D"), + arrival_time = c("08:00:00", "08:09:30", "08:19:30", "08:40:00"), + departure_time = c("08:00:00", "08:10:30", "08:20:30", "08:40:00") + ) + ) + expect_identical( + trip_times(new_gtfs, "u"), + data.table::data.table( + stop_id = c("W", "X", "Y", "Z"), + arrival_time = c("", "07:00:00", "07:09:30", "07:20:00"), + departure_time = c("", "07:00:00", "07:10:30", "07:20:00") + ) + ) + + # the default shifts the later times instead, at every stop + + new_gtfs <- tester(four_gtfs, trip_id = "t", dwell_time = 60) + expect_identical( + trip_times(new_gtfs, "t")$arrival_time, + c("08:00:00", "08:11:00", "08:22:00", "08:43:00") + ) + + # the first and last stops are never changed, silently + + expect_silent( + new_gtfs <- tester( + four_gtfs, + stop_id = c("A", "D"), + strategy = "keep_total_duration" + ) + ) + expect_identical(new_gtfs$stop_times, four_gtfs$stop_times) + + # in a loop, the first visit to A is skipped, but the later one is changed + + new_gtfs <- tester( + hand_gtfs, + stop_id = "A", + dwell_time = 120, + strategy = "keep_total_duration" + ) + expect_identical( + trip_times(new_gtfs, "t"), + data.table::data.table( + stop_id = c("A", "B", "C", "A", "D"), + arrival_time = c("23:59:00", "24:05:00", "", "24:09:00", "24:20:00"), + departure_time = c("24:00:00", "24:05:30", "", "24:11:00", "24:21:00") + ) + ) +}) + +test_that("keep_total_duration decreases dwell times and splits odd seconds", { + v_gtfs <- speed_gtfs( + v = list( + stop_id = c("A", "B", "C", "D"), + arr = c("08:00:00", "08:10:00", "08:20:00", "08:30:00"), + dep = c("08:00:00", "08:12:00", "08:20:00", "08:30:00") + ) + ) + + # B's 120 seconds dwell time is removed around its middle + + new_gtfs <- tester( + v_gtfs, + stop_id = "B", + dwell_time = 0, + strategy = "keep_total_duration" + ) + expect_identical( + trip_times(new_gtfs, "v")[stop_id == "B", -1], + data.table::data.table( + arrival_time = "08:11:00", + departure_time = "08:11:00" + ) + ) + + # an odd second goes to the departure, whether the change is positive (C, + # +61) or negative (B, -61) + + new_gtfs <- tester( + v_gtfs, + stop_id = "C", + dwell_time = 61, + strategy = "keep_total_duration" + ) + expect_identical( + trip_times(new_gtfs, "v")[stop_id == "C", -1], + data.table::data.table( + arrival_time = "08:19:30", + departure_time = "08:20:31" + ) + ) + + new_gtfs <- tester( + v_gtfs, + stop_id = "B", + dwell_time = 59, + strategy = "keep_total_duration" + ) + expect_identical( + trip_times(new_gtfs, "v")[stop_id == "B", -1], + data.table::data.table( + arrival_time = "08:10:30", + departure_time = "08:11:29" + ) + ) +}) + +test_that("keep_total_duration raises an error if running times go negative", { + + # B is blank, so C's arrival is compared with A's departure + + w_gtfs <- speed_gtfs( + w = list( + stop_id = c("A", "B", "C", "D"), + arr = c("08:00:00", "", "08:02:00", "08:10:00"), + dep = c("08:00:00", "", "08:02:00", "08:10:00") + ) + ) + original_stop_times <- data.table::copy(w_gtfs$stop_times) + + new_gtfs <- tester( + w_gtfs, + stop_id = "C", + dwell_time = 240, + strategy = "keep_total_duration" + ) + expect_identical( + trip_times(new_gtfs, "w")[stop_id == "C"]$arrival_time, + "08:00:00" + ) + + err <- expect_error( + tester( + w_gtfs, + stop_id = "C", + dwell_time = 242, + strategy = "keep_total_duration", + by_reference = TRUE + ), + class = "gtfstools_negative_running_time" + ) + expect_identical(err$call[[1]], quote(set_dwell_time)) + expect_identical(w_gtfs$stop_times, original_stop_times) + + # running times that are already negative don't raise this error, but times + # out of range still raise their own + + n_gtfs <- speed_gtfs( + n = list( + stop_id = c("A", "B", "C", "D"), + arr = c("00:00:00", "00:00:10", "00:10:00", "00:20:00"), + dep = c("00:05:00", "00:00:10", "00:10:00", "00:20:00") + ) + ) + + new_gtfs <- tester( + n_gtfs, + stop_id = "B", + dwell_time = 20, + strategy = "keep_total_duration" + ) + expect_identical( + trip_times(new_gtfs, "n")[stop_id == "B", -1], + data.table::data.table( + arrival_time = "00:00:00", + departure_time = "00:00:20" + ) + ) + + expect_error( + tester( + n_gtfs, + stop_id = "B", + dwell_time = 60, + strategy = "keep_total_duration" + ), + class = "gtfstools_time_out_of_range" + ) +}) + +test_that("keep_total_duration works on a real feed with _secs columns", { + original_gtfs <- read_gtfs(spo_path) + spo_gtfs <- read_gtfs(spo_path) + trip <- "CPTM L07-0" + + new_gtfs <- tester(spo_gtfs, trip_id = trip, strategy = "keep_total_duration") + expect_s3_class(new_gtfs, "dt_gtfs") + expect_equal(original_gtfs, spo_gtfs, ignore_attr = TRUE) + + # the first departure (and so the departures in 'frequencies') and the last + # arrival are kept, and every intermediate dwell time is set + + old_times <- trip_times(spo_gtfs, trip) + new_times <- trip_times(new_gtfs, trip) + n <- nrow(new_times) + expect_identical(new_times[c(1, n)], old_times[c(1, n)]) + expect_true( + all(get_dwell_time(new_gtfs, trip)$dwell_time[-c(1, n)] == 30) + ) + + secs_gtfs <- convert_time_to_seconds(spo_gtfs) + st <- tester( + secs_gtfs, + trip_id = trip, + strategy = "keep_total_duration" + )$stop_times + expect_equal(st$arrival_time_secs, string_to_seconds(st$arrival_time)) + expect_equal(st$departure_time_secs, string_to_seconds(st$departure_time)) +}) From 56cfed9e033520aa7f24ef8ab14b49c1180e148f Mon Sep 17 00:00:00 2001 From: rafapereirabr Date: Sun, 1 Nov 2026 13:55:51 -0300 Subject: [PATCH 2/6] new function set_min_speed() set_min_speed() raises the speed of trip spans slower than min_speed, shifting the later times of each trip earlier and keeping dwell times. It mirrors set_max_speed(): both now call the internal limit_trip_speed() helper, whose behaviour in max mode is unchanged. Argument checks stay in the exported functions, and warn_missing_ids() gains a call argument, so warnings and errors are still attributed to the exported functions. Co-Authored-By: Claude Opus 5.5 --- NAMESPACE | 1 + NEWS.md | 2 +- R/set_max_speed.R | 139 +++++++++++--- R/set_min_speed.R | 149 +++++++++++++++ R/utils.R | 10 +- _pkgdown.yml | 1 + man/set_max_speed.Rd | 2 +- man/set_min_speed.Rd | 137 +++++++++++++ man/warn_missing_ids.Rd | 5 +- tests/testthat/test-set_max_speed.R | 8 + tests/testthat/test-set_min_speed.R | 287 ++++++++++++++++++++++++++++ 11 files changed, 708 insertions(+), 33 deletions(-) create mode 100644 R/set_min_speed.R create mode 100644 man/set_min_speed.Rd create mode 100644 tests/testthat/test-set_min_speed.R diff --git a/NAMESPACE b/NAMESPACE index 6d360b36..8b3d4e33 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -66,6 +66,7 @@ export(remove_unused_ids) export(set_dwell_time) export(set_max_speed) export(set_min_dwell_time) +export(set_min_speed) export(set_route_frequency) export(set_route_speed) export(set_trip_speed) diff --git a/NEWS.md b/NEWS.md index 5d1f099b..76d3b54d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -45,7 +45,7 @@ - New functions to edit feeds: - `set_route_frequency()` sets the headway of routes within a time of day, replacing their trips in it with a frequency-based template trip. Departures outside the time of day don't change. - `set_route_speed()` sets the average speed of all trips of the given routes, as `set_trip_speed()` does for trips. - - `set_dwell_time()`, `set_min_dwell_time()` and `set_max_speed()` set dwell times, raise dwell times at intermediate stops to a minimum and cap trip speeds, shifting the later times of each trip accordingly. The dwell times can be set within a time of day, and speeds capped only between two stops or for trips departing within a time of day, as in `set_trip_speed()`. Like `set_trip_speed()` and `set_route_speed()`, they and `set_route_frequency()` can be restricted to some transport modes with `route_type`. + - `set_dwell_time()`, `set_min_dwell_time()`, `set_max_speed()` and `set_min_speed()` set dwell times, raise dwell times at intermediate stops to a minimum, and cap or raise trip speeds to a maximum or minimum, shifting the later times of each trip accordingly. The dwell times can be set within a time of day, and speeds changed only between two stops or for trips departing within a time of day, as in `set_trip_speed()`. Like `set_trip_speed()` and `set_route_speed()`, they and `set_route_frequency()` can be restricted to some transport modes with `route_type`. - `interpolate_stop_times()` fills blank arrival and departure times, assuming a constant speed between stops with known times. - `interpolate_shape_dist_traveled()` (re)calculates `shape_dist_traveled` in `stop_times` and `shapes`, in meters. - `stop_times_to_frequencies()`, the counterpart of `frequencies_to_stop_times()`, approximates scheduled trips by frequency-based ones (#69). diff --git a/R/set_max_speed.R b/R/set_max_speed.R index 3a7e2c9d..c06fc248 100644 --- a/R/set_max_speed.R +++ b/R/set_max_speed.R @@ -64,7 +64,7 @@ #' `"24:00:00"` for departures after midnight. Trips without a time there are #' left unchanged. #' -#' @seealso [set_trip_speed()], [get_trip_speed()] +#' @seealso [set_min_speed()], [set_trip_speed()], [get_trip_speed()] #' #' @examples #' \dontshow{ @@ -137,11 +137,52 @@ set_max_speed <- function( ) window <- dwell_time_window(from, to) + + # calculations use km/h + + max_kmh <- max_speed * speed_unit_factors[[unit]] + + return( + limit_trip_speed( + gtfs, + max_kmh, + "max", + route_id, + trip_id, + first_stop, + last_stop, + window, + route_type + ) + ) +} + + + +# changes the speed of the selected trips, either capping it ('mode = "max"', +# used by set_max_speed()) or raising it ('mode = "min"', used by +# set_min_speed()) to 'limit_kmh'. the arguments are validated by the callers, +# so that their errors are attributed to them, and 'window' is the output of +# dwell_time_window() + +limit_trip_speed <- function( + gtfs, + limit_kmh, + mode, + route_id, + trip_id, + first_stop, + last_stop, + window, + route_type +) { + is_max <- identical(mode, "max") + from_secs <- window[1] to_secs <- window[2] has_stop_args <- !is.null(first_stop) || !is.null(last_stop) - has_window <- !is.null(from) || !is.null(to) + has_window <- any(is.finite(window)) gtfsio::assert_field_class( gtfs, @@ -154,10 +195,6 @@ set_max_speed <- function( gtfsio::assert_field_class(gtfs, "stop_times", "stop_id", "character") } - # calculations use km/h - - max_kmh <- max_speed * speed_unit_factors[[unit]] - # select the trips. ids is NULL only when both arguments are NULL, so that # trip_ids that don't exist don't result in all trips being changed @@ -167,12 +204,16 @@ set_max_speed <- function( if (!is.null(trip_id) || !is.null(route_id)) { ids <- character(0) + # the call must be evaluated directly in this function's body, so that + # the warning is attributed to its caller + if (!is.null(trip_id)) { invalid_trip_id <- warn_missing_ids( trip_id, existing_trips, "stop_times", - "trip_id" + "trip_id", + call = sys.call(-1) ) ids <- setdiff(trip_id, invalid_trip_id) } @@ -344,10 +385,22 @@ set_max_speed <- function( span_len := i.span_len ] + # capped spans last at least their length divided by the limit, rounded up, + # and raised ones at most that, rounded down, so that their speed respects + # the limit. raising spans of length 0 would remove the time spent between + # stops at the same place, so they are left unchanged. the extra time of + # raised spans is negative + is_first <- data.table::rowid(timed$trip_id) == 1L span_dur <- timed$t_arr - data.table::shift(timed$t_dep) - min_dur <- ceiling(timed$span_len / max_kmh * 3600) - extra <- pmax(min_dur - span_dur, 0) + if (is_max) { + min_dur <- ceiling(timed$span_len / limit_kmh * 3600) + extra <- pmax(min_dur - span_dur, 0) + } else { + max_dur <- floor(timed$span_len / limit_kmh * 3600) + extra <- -pmax(span_dur - max_dur, 0) + extra[!is.na(timed$span_len) & timed$span_len <= 0] <- 0 + } extra[is_first | is.na(extra)] <- 0 # spans that end before they start have invalid times, which are not @@ -375,12 +428,13 @@ set_max_speed <- function( if (any(is_backwards)) { backwards_trips <- unique(timed$trip_id[is_backwards]) abbreviated <- abbreviate_ids(backwards_trips) + changed <- if (is_max) "capped" else "raised" cli::cli_warn( c( paste0( "{length(backwards_trips)} trip{?s} {?has/have} stops whose ", "arrival is earlier than the previous departure, so {?its/their} ", - "speed{?s} {?was/were} not capped between them: ", + "speed{?s} {?was/were} not ", changed, " between them: ", "{.val {abbreviated$shown}}{abbreviated$more}." ), "i" = paste0( @@ -392,36 +446,65 @@ set_max_speed <- function( ) } - # the delay of each stop is the extra time accumulated up to it in its trip. - # shifting a span doesn't change the duration of the following ones, so the - # delays don't depend on each other + # the delay of each stop is the extra time accumulated up to it in its trip + # (negative when raising speeds). shifting a span doesn't change the + # duration of the following ones, so the delays don't depend on each other cum_extra <- cumsum(extra) trip_start <- cumsum(is_first) timed[, delay := cum_extra - (cum_extra - extra)[is_first][trip_start]] - timed <- timed[delay > 0] + timed <- timed[delay != 0] if (nrow(timed) == 0) { return(gtfs) } - # seconds_to_string() can't write times later than 9999:59:59 + # seconds_to_string() can't write times later than 9999:59:59 or earlier + # than 00:00:00. the latter usually happens after a span that ends before + # it starts. the calls to cli_abort() must be evaluated directly in this + # function's body, so that the errors are attributed to its caller - is_invalid <- pmax(timed$arr, timed$dep, na.rm = TRUE) + timed$delay > - max_time_secs + if (is_max) { + is_invalid <- pmax(timed$arr, timed$dep, na.rm = TRUE) + timed$delay > + max_time_secs - if (any(is_invalid)) { - invalid_trips <- unique(timed$trip_id[is_invalid]) - abbreviated <- abbreviate_ids(invalid_trips) - cli::cli_abort( - paste0( - "Capping the speed results in times later than {.val 9999:59:59} ", - "in the following {cli::qty(length(invalid_trips))}trip{?s}: ", - "{.val {abbreviated$shown}}{abbreviated$more}." - ), - class = "gtfstools_time_out_of_range" - ) + if (any(is_invalid)) { + invalid_trips <- unique(timed$trip_id[is_invalid]) + abbreviated <- abbreviate_ids(invalid_trips) + cli::cli_abort( + paste0( + "Capping the speed results in times later than {.val 9999:59:59} ", + "in the following {cli::qty(length(invalid_trips))}trip{?s}: ", + "{.val {abbreviated$shown}}{abbreviated$more}." + ), + class = "gtfstools_time_out_of_range", + call = parent.frame() + ) + } + } else { + is_invalid <- pmin(timed$arr, timed$dep, na.rm = TRUE) + timed$delay < 0 + + if (any(is_invalid)) { + invalid_trips <- unique(timed$trip_id[is_invalid]) + abbreviated <- abbreviate_ids(invalid_trips) + cli::cli_abort( + c( + paste0( + "Raising the speed results in times earlier than ", + "{.val 00:00:00} in the following ", + "{cli::qty(length(invalid_trips))}trip{?s}: ", + "{.val {abbreviated$shown}}{abbreviated$more}." + ), + "i" = paste0( + "This usually happens when times after midnight are written as ", + "{.val 00:30:00} instead of {.val 24:30:00}." + ) + ), + class = "gtfstools_time_out_of_range", + call = parent.frame() + ) + } } # shift the non-blank times of the delayed stops diff --git a/R/set_min_speed.R b/R/set_min_speed.R new file mode 100644 index 00000000..734fa99b --- /dev/null +++ b/R/set_min_speed.R @@ -0,0 +1,149 @@ +#' Set trip minimum speed +#' +#' Raises the speed of the specified trips: each part of a trip that is slower +#' than `min_speed` is sped up to `min_speed`, by changing the `arrival_time` +#' and `departure_time` columns in `stop_times`. Parts of the trips that are +#' already faster are left unchanged. +#' +#' @template gtfs +#' @param min_speed A number. The minimum speed allowed, in the unit given by +#' `unit`. +#' @param route_id A character vector including the `route_id`s whose trips +#' have their speeds raised. If `route_id`, `trip_id` and `route_type` are +#' all `NULL` (the default), the speeds of every trip in the GTFS are raised. +#' `route_id`s not listed in `trips` are ignored, with a warning. +#' @param trip_id A character vector including the `trip_id`s to have their +#' speeds raised. When given together with `route_id`, the trips of both are +#' raised. +#' @param first_stop A string. The `stop_id` where the segment whose speeds +#' are raised starts. If `NULL` (the default), the segment starts at each +#' trip's first stop. +#' @param last_stop A string. The `stop_id` where the segment ends. If `NULL` +#' (the default), the segment ends at each trip's last stop. +#' @inheritParams set_max_speed +#' +#' @return A GTFS object with the time columns of its `stop_times` adjusted. +#' +#' @section Details: +#' A span is the part of a trip between two consecutive stops with times; +#' stops with blank times inside it are kept blank. Its speed is its length, +#' calculated by [get_trip_length()] along the trip's shape (or, with a +#' warning, as straight lines between stops if there are no shapes; the +#' `stops` table is required), divided by the time from the departure at its +#' first stop to the arrival at its second. Spans whose length is `NA` (e.g. +#' in trips not linked to a shape) or 0 (e.g. between stops at the same +#' place) are left unchanged. +#' +#' Each span slower than `min_speed` has its duration lowered to its length +#' divided by `min_speed`, rounded down to the whole second (so a very +#' short span may end up with a duration of 0). All the later times of the +#' trip are shifted earlier by the accumulated saved time, so dwell times are +#' kept. Spans whose arrival is earlier than the previous departure (usually +#' times after midnight written as `"00:30:00"` instead of `"24:30:00"`) are +#' left unchanged, with a warning. An error is raised if a shift would result +#' in a time earlier than `"00:00:00"`. Only `stop_times` is modified; its +#' existing `_secs` columns are updated to match. +#' +#' With `first_stop` and `last_stop`, matched as in [set_trip_speed()], only +#' the spans between them are raised: times before `first_stop` don't change, +#' and the times at and after `last_stop` are shifted. The given stops must +#' have a time, so that no span crosses them (if only `last_stop` is given, +#' the trip's first stop needs none). Trips that don't visit `first_stop` and +#' then `last_stop`, or have blank times there, are left unchanged, with a +#' warning if they were given in `trip_id` or `route_id`. +#' +#' `from` and `to` are compared with the departure at the segment's first stop +#' (or its arrival, if the departure is blank) as written in `stop_times` +#' (template times, for trips listed in `frequencies`), so use times past +#' `"24:00:00"` for departures after midnight. Trips without a time there are +#' left unchanged. +#' +#' @seealso [set_max_speed()], [set_trip_speed()], [get_trip_speed()] +#' +#' @examples +#' \dontshow{ +#' old_dt_threads <- data.table::setDTthreads(1) +#' on.exit(data.table::setDTthreads(old_dt_threads), add = TRUE) +#' } +#' data_path <- system.file("extdata/spo_gtfs.zip", package = "gtfstools") +#' +#' gtfs <- read_gtfs(data_path) +#' +#' # the trip is slower than 20 km/h between some of its stops +#' get_trip_speed(gtfs, "CPTM L07-0", by = "segment") +#' +#' gtfs_raised <- set_min_speed(gtfs, min_speed = 20, trip_id = "CPTM L07-0") +#' get_trip_speed(gtfs_raised, "CPTM L07-0", by = "segment") +#' +#' # the later stops are shifted, so the trip ends earlier +#' get_start_and_end_times(gtfs, trip_id = "CPTM L07-0") +#' get_start_and_end_times(gtfs_raised, trip_id = "CPTM L07-0") +#' +#' # raise the speed only between two stops. the spans before and after them +#' # are not raised +#' gtfs_raised <- set_min_speed( +#' gtfs, +#' min_speed = 20, +#' trip_id = "CPTM L07-0", +#' first_stop = "18917", +#' last_stop = "18922" +#' ) +#' get_trip_speed(gtfs_raised, "CPTM L07-0", by = "segment") +#' +#' @export +set_min_speed <- function( + gtfs, + min_speed, + unit = "km/h", + route_id = NULL, + trip_id = NULL, + first_stop = NULL, + last_stop = NULL, + from = NULL, + to = NULL, + route_type = NULL +) { + gtfs <- assert_and_assign_gtfs_object(gtfs) + checkmate::assert_number( + min_speed, + lower = .Machine$double.eps, + finite = TRUE + ) + checkmate::assert_character(route_id, null.ok = TRUE, any.missing = FALSE) + checkmate::assert_character(trip_id, null.ok = TRUE, any.missing = FALSE) + checkmate::assert( + checkmate::check_string(unit), + checkmate::check_names(unit, subset.of = names(speed_unit_factors)), + combine = "and" + ) + checkmate::assert_string(first_stop, min.chars = 1, null.ok = TRUE) + checkmate::assert_string(last_stop, min.chars = 1, null.ok = TRUE) + checkmate::assert_string(from, pattern = time_pattern, null.ok = TRUE) + checkmate::assert_string(to, pattern = time_pattern, null.ok = TRUE) + checkmate::assert_integerish( + route_type, + min.len = 1, + any.missing = FALSE, + null.ok = TRUE + ) + + window <- dwell_time_window(from, to) + + # calculations use km/h + + min_kmh <- min_speed * speed_unit_factors[[unit]] + + return( + limit_trip_speed( + gtfs, + min_kmh, + "min", + route_id, + trip_id, + first_stop, + last_stop, + window, + route_type + ) + ) +} diff --git a/R/utils.R b/R/utils.R index f67c383a..9cc25f77 100644 --- a/R/utils.R +++ b/R/utils.R @@ -230,11 +230,17 @@ tables_not_data_table <- function(gtfs) { #' @param existing_id A character vector of the ids that exist in the table. #' @param table The name of the table, used in the warning message. #' @param id_name The name of the id field, used in the warning message. +#' @param call The call the warning is attributed to. Defaults to the call of +#' the function that called this helper. #' #' @return The missing ids, invisibly. #' #' @keywords internal -warn_missing_ids <- function(id, existing_id, table, id_name) { +warn_missing_ids <- function(id, + existing_id, + table, + id_name, + call = sys.call(-1)) { invalid_id <- id[! id %chin% existing_id] if (length(invalid_id) > 0) { @@ -242,7 +248,7 @@ warn_missing_ids <- function(id, existing_id, table, id_name) { "'", table, "' doesn't contain the following ", id_name, "(s): ", paste0("'", invalid_id, "'", collapse = ", ") ) - warning(simpleWarning(msg, call = sys.call(-1))) + warning(simpleWarning(msg, call = call)) } return(invisible(invalid_id)) diff --git a/_pkgdown.yml b/_pkgdown.yml index 7258df69..e8b49ffc 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -44,6 +44,7 @@ reference: - set_dwell_time - set_min_dwell_time - set_max_speed + - set_min_speed - set_route_frequency - set_trip_speed - set_route_speed diff --git a/man/set_max_speed.Rd b/man/set_max_speed.Rd index fc178b6b..4306cfe2 100644 --- a/man/set_max_speed.Rd +++ b/man/set_max_speed.Rd @@ -134,5 +134,5 @@ gtfs_capped <- set_max_speed(gtfs, max_speed = 8) } \seealso{ -\code{\link[=set_trip_speed]{set_trip_speed()}}, \code{\link[=get_trip_speed]{get_trip_speed()}} +\code{\link[=set_min_speed]{set_min_speed()}}, \code{\link[=set_trip_speed]{set_trip_speed()}}, \code{\link[=get_trip_speed]{get_trip_speed()}} } diff --git a/man/set_min_speed.Rd b/man/set_min_speed.Rd new file mode 100644 index 00000000..ebdcb37e --- /dev/null +++ b/man/set_min_speed.Rd @@ -0,0 +1,137 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/set_min_speed.R +\name{set_min_speed} +\alias{set_min_speed} +\title{Set trip minimum speed} +\usage{ +set_min_speed( + gtfs, + min_speed, + unit = "km/h", + route_id = NULL, + trip_id = NULL, + first_stop = NULL, + last_stop = NULL, + from = NULL, + to = NULL, + route_type = NULL +) +} +\arguments{ +\item{gtfs}{A GTFS object, as created by \code{\link[=read_gtfs]{read_gtfs()}}.} + +\item{min_speed}{A number. The minimum speed allowed, in the unit given by +\code{unit}.} + +\item{unit}{A string representing the unit in which the speed is given. One +of \code{"km/h"} (the default), \code{"m/s"} or \code{"mph"} (miles per hour).} + +\item{route_id}{A character vector including the \code{route_id}s whose trips +have their speeds raised. If \code{route_id}, \code{trip_id} and \code{route_type} are +all \code{NULL} (the default), the speeds of every trip in the GTFS are raised. +\code{route_id}s not listed in \code{trips} are ignored, with a warning.} + +\item{trip_id}{A character vector including the \code{trip_id}s to have their +speeds raised. When given together with \code{route_id}, the trips of both are +raised.} + +\item{first_stop}{A string. The \code{stop_id} where the segment whose speeds +are raised starts. If \code{NULL} (the default), the segment starts at each +trip's first stop.} + +\item{last_stop}{A string. The \code{stop_id} where the segment ends. If \code{NULL} +(the default), the segment ends at each trip's last stop.} + +\item{from}{A string, in the "HH:MM:SS" format. Only trips that depart from +the segment's first stop at or after \code{from} are changed. If \code{NULL} (the +default), there is no lower limit.} + +\item{to}{A string, in the "HH:MM:SS" format. Only trips that depart from +the segment's first stop at or before \code{to} are changed. If \code{NULL} (the +default), there is no upper limit.} + +\item{route_type}{An integer vector of \code{route_type}s (see +\code{\link[=filter_by_route_type]{filter_by_route_type()}}). If given, only trips of routes of these types +are changed; together with the other selection arguments, it restricts +their selection. \code{route_type}s not listed in \code{routes} are ignored. If +\code{NULL} (the default), trips of any type are changed.} +} +\value{ +A GTFS object with the time columns of its \code{stop_times} adjusted. +} +\description{ +Raises the speed of the specified trips: each part of a trip that is slower +than \code{min_speed} is sped up to \code{min_speed}, by changing the \code{arrival_time} +and \code{departure_time} columns in \code{stop_times}. Parts of the trips that are +already faster are left unchanged. +} +\section{Details}{ + +A span is the part of a trip between two consecutive stops with times; +stops with blank times inside it are kept blank. Its speed is its length, +calculated by \code{\link[=get_trip_length]{get_trip_length()}} along the trip's shape (or, with a +warning, as straight lines between stops if there are no shapes; the +\code{stops} table is required), divided by the time from the departure at its +first stop to the arrival at its second. Spans whose length is \code{NA} (e.g. +in trips not linked to a shape) or 0 (e.g. between stops at the same +place) are left unchanged. + +Each span slower than \code{min_speed} has its duration lowered to its length +divided by \code{min_speed}, rounded down to the whole second (so a very +short span may end up with a duration of 0). All the later times of the +trip are shifted earlier by the accumulated saved time, so dwell times are +kept. Spans whose arrival is earlier than the previous departure (usually +times after midnight written as \code{"00:30:00"} instead of \code{"24:30:00"}) are +left unchanged, with a warning. An error is raised if a shift would result +in a time earlier than \code{"00:00:00"}. Only \code{stop_times} is modified; its +existing \verb{_secs} columns are updated to match. + +With \code{first_stop} and \code{last_stop}, matched as in \code{\link[=set_trip_speed]{set_trip_speed()}}, only +the spans between them are raised: times before \code{first_stop} don't change, +and the times at and after \code{last_stop} are shifted. The given stops must +have a time, so that no span crosses them (if only \code{last_stop} is given, +the trip's first stop needs none). Trips that don't visit \code{first_stop} and +then \code{last_stop}, or have blank times there, are left unchanged, with a +warning if they were given in \code{trip_id} or \code{route_id}. + +\code{from} and \code{to} are compared with the departure at the segment's first stop +(or its arrival, if the departure is blank) as written in \code{stop_times} +(template times, for trips listed in \code{frequencies}), so use times past +\code{"24:00:00"} for departures after midnight. Trips without a time there are +left unchanged. +} + +\examples{ +\dontshow{ + old_dt_threads <- data.table::setDTthreads(1) + on.exit(data.table::setDTthreads(old_dt_threads), add = TRUE) +} +data_path <- system.file("extdata/spo_gtfs.zip", package = "gtfstools") + +gtfs <- read_gtfs(data_path) + +# the trip is slower than 20 km/h between some of its stops +get_trip_speed(gtfs, "CPTM L07-0", by = "segment") + +gtfs_raised <- set_min_speed(gtfs, min_speed = 20, trip_id = "CPTM L07-0") +get_trip_speed(gtfs_raised, "CPTM L07-0", by = "segment") + +# the later stops are shifted, so the trip ends earlier +get_start_and_end_times(gtfs, trip_id = "CPTM L07-0") +get_start_and_end_times(gtfs_raised, trip_id = "CPTM L07-0") + +# raise the speed only between two stops. the spans before and after them +# are not raised +gtfs_raised <- set_min_speed( + gtfs, + min_speed = 20, + trip_id = "CPTM L07-0", + first_stop = "18917", + last_stop = "18922" +) +get_trip_speed(gtfs_raised, "CPTM L07-0", by = "segment") + +} +\seealso{ +\code{\link[=set_max_speed]{set_max_speed()}}, \code{\link[=set_trip_speed]{set_trip_speed()}}, \code{\link[=get_trip_speed]{get_trip_speed()}} +} diff --git a/man/warn_missing_ids.Rd b/man/warn_missing_ids.Rd index 72a37e39..2a08d636 100644 --- a/man/warn_missing_ids.Rd +++ b/man/warn_missing_ids.Rd @@ -4,7 +4,7 @@ \alias{warn_missing_ids} \title{Warn about ids missing from a table} \usage{ -warn_missing_ids(id, existing_id, table, id_name) +warn_missing_ids(id, existing_id, table, id_name, call = sys.call(-1)) } \arguments{ \item{id}{A character vector of ids given by the user.} @@ -14,6 +14,9 @@ warn_missing_ids(id, existing_id, table, id_name) \item{table}{The name of the table, used in the warning message.} \item{id_name}{The name of the id field, used in the warning message.} + +\item{call}{The call the warning is attributed to. Defaults to the call of +the function that called this helper.} } \value{ The missing ids, invisibly. diff --git a/tests/testthat/test-set_max_speed.R b/tests/testthat/test-set_max_speed.R index 4b618044..7f0f10fc 100644 --- a/tests/testthat/test-set_max_speed.R +++ b/tests/testthat/test-set_max_speed.R @@ -167,6 +167,14 @@ test_that("raises an error if times would be later than 9999:59:59", { ) }) +test_that("attributes warnings and errors to set_max_speed()", { + w <- tryCatch(tester(trip_id = "ola"), warning = function(w) w) + expect_identical(conditionCall(w)[[1]], quote(set_max_speed)) + + e <- tryCatch(tester(max_speed = 1e-6), error = function(e) e) + expect_identical(conditionCall(e)[[1]], quote(set_max_speed)) +}) + test_that("results in a dt_gtfs object", { result <- tester() expect_s3_class(result, "dt_gtfs") diff --git a/tests/testthat/test-set_min_speed.R b/tests/testthat/test-set_min_speed.R new file mode 100644 index 00000000..01f46b57 --- /dev/null +++ b/tests/testthat/test-set_min_speed.R @@ -0,0 +1,287 @@ +spo_path <- system.file("extdata/spo_gtfs.zip", package = "gtfstools") +spo_gtfs <- read_gtfs(spo_path) + +tester <- function(gtfs = hand_gtfs, + min_speed = 30, + route_id = NULL, + trip_id = NULL, + unit = "km/h", + ...) { + set_min_speed(gtfs, min_speed, unit, route_id, trip_id, ...) +} + +# creates a feed whose trips serve the same stops on the equator, along a +# straight shape. 'times' is a list with the arrival and departure times of +# each trip + +synthetic_feed <- function(times, route_id, stop_id = paste0("s", 1:5)) { + trip_ids <- paste0("t", seq_along(times)) + + feed <- list( + trips = data.table::data.table( + trip_id = trip_ids, + route_id = route_id, + shape_id = "sh1" + ), + shapes = data.table::data.table( + shape_id = "sh1", + shape_pt_lat = c(0, 0), + shape_pt_lon = c(0, 0.04), + shape_pt_sequence = 1:2 + ), + stops = data.table::data.table( + stop_id = paste0("s", 1:5), + stop_lat = 0, + stop_lon = c(0, 0.01, 0.02, 0.03, 0.04) + ), + stop_times = data.table::rbindlist( + lapply( + seq_along(times), + function(i) { + data.table::data.table( + trip_id = trip_ids[i], + arrival_time = times[[i]]$arr, + departure_time = times[[i]]$dep, + stop_id = stop_id, + stop_sequence = seq_along(stop_id) + ) + } + ) + ) + ) + + return(gtfsio::new_gtfs(feed, "dt_gtfs")) +} + +secs <- function(x) string_to_seconds(x) +times_of <- function(gtfs, trip) { + gtfs$stop_times[trip_id == trip, .(arrival_time, departure_time)] +} +same_times <- function(x) list(arr = x, dep = x) + +slow <- same_times( + c("08:00:00", "08:10:00", "08:20:00", "08:30:00", "08:40:00") +) + +# maximum duration, in seconds, of spans with one and two segments at 30 km/h + +segment_km <- get_trip_length( + synthetic_feed(list(slow), "r1"), + by = "segment" +)$length[1] +max_one <- as.integer(floor(segment_km / 30 * 3600)) +max_two <- as.integer(floor(2 * segment_km / 30 * 3600)) + +# a span just faster than 30 km/h, one with zero duration, one just slower and +# a fast one + +boundary_secs <- cumsum(c(8 * 3600, max_one, 0L, max_one + 1L, 60L)) + +hand_gtfs <- synthetic_feed( + list( + # slow first span, a blank stop inside a slow span and a fast last span + list( + arr = c("08:00:00", "08:10:00", "", "08:30:00", "08:31:00"), + dep = c("08:00:00", "08:11:00", "", "08:30:30", "08:31:00") + ), + same_times(seconds_to_string(as.integer(boundary_secs))), + slow + ), + route_id = c("r1", "r1", "r2") +) + +# tests --- + +test_that("raises error due to incorrect input types", { + expect_error(tester(unclass(hand_gtfs))) + expect_error(tester(min_speed = "30")) + expect_error(tester(min_speed = 0)) + expect_error(tester(min_speed = -1)) + expect_error(tester(min_speed = NA_real_)) + expect_error(tester(min_speed = Inf)) + expect_error(tester(min_speed = c(30, 40))) + expect_error(tester(unit = "kms/h")) + expect_error(tester(unit = c("km/h", "m/s"))) + expect_error(tester(trip_id = NA_character_)) + expect_error(tester(trip_id = 1)) + expect_error(tester(route_id = NA_character_)) + expect_error(tester(route_id = 1)) + expect_error(tester(first_stop = "")) + expect_error(tester(last_stop = c("s1", "s2"))) + expect_error(tester(from = "8:00:00")) + expect_error(tester(to = "8:00")) + expect_error(tester(route_type = "3")) + + e <- tryCatch( + tester(from = "09:00:00", to = "08:00:00"), + error = function(e) e + ) + expect_s3_class(e, "gtfstools_invalid_time_of_day") + expect_identical(conditionCall(e)[[1]], quote(set_min_speed)) +}) + +test_that("raises warnings if non-existent ids are given", { + w <- tryCatch(tester(trip_id = c("t1", "ola")), warning = function(w) w) + expect_match(conditionMessage(w), "ola") + expect_identical(conditionCall(w)[[1]], quote(set_min_speed)) + + # an unknown trip_id doesn't result in all trips being changed + + expect_warning(result <- tester(trip_id = "ola")) + expect_identical(result, hand_gtfs) +}) + +test_that("results in a dt_gtfs object", { + result <- tester() + expect_s3_class(result, "dt_gtfs") + expect_s3_class(result, "gtfs") + expect_type(result, "list") + expect_s3_class(result$stop_times, "data.table") +}) + +test_that("doesn't change given gtfs", { + original_gtfs <- read_gtfs(spo_path) + gtfs <- read_gtfs(spo_path) + expect_identical(original_gtfs, gtfs) + + result <- tester(gtfs, min_speed = 20, trip_id = "CPTM L07-0") + expect_false(identical(result$stop_times, gtfs$stop_times)) + expect_equal(original_gtfs, gtfs, ignore_attr = TRUE) + + original_hand <- data.table::copy(hand_gtfs$stop_times) + result <- tester() + expect_identical(hand_gtfs$stop_times, original_hand) +}) + +test_that("raises the speed of every trip by default", { + result <- tester(spo_gtfs, min_speed = 15) + + # segments of length 0 keep their speed of 0 + + speeds <- get_trip_speed(result, by = "segment") + expect_true(all(speeds[speed > 0]$speed >= 15 - 1e-8, na.rm = TRUE)) + + original_speeds <- get_trip_speed(spo_gtfs, by = "segment") + expect_true(any(original_speeds[speed > 0]$speed < 15, na.rm = TRUE)) +}) + +test_that("speeds up slow spans and shifts the later times earlier", { + result <- tester(trip_id = "t1") + times <- times_of(result, "t1") + + # the saved times of the first two spans accumulate. the dwell times are + # kept and the blank stop inside the slow span stays blank + + saved_first <- 600L - max_one + saved_second <- 1140L - max_two + + expect_identical( + secs(times$arrival_time), + secs(times_of(hand_gtfs, "t1")$arrival_time) - + c(0L, saved_first, NA, rep(saved_first + saved_second, 2)) + ) + expect_identical( + secs(times$departure_time), + secs(times_of(hand_gtfs, "t1")$departure_time) - + c(0L, saved_first, NA, rep(saved_first + saved_second, 2)) + ) + expect_identical(times$arrival_time[3], "") + + # the other trips don't change + + expect_identical( + result$stop_times[trip_id != "t1"], + hand_gtfs$stop_times[trip_id != "t1"] + ) +}) + +test_that("changes only the spans slower than min_speed", { + result <- tester(trip_id = "t2") + + # only the span just slower than 30 km/h is shortened, by a second + + expect_identical( + secs(times_of(result, "t2")$arrival_time), + as.integer(boundary_secs) - c(0L, 0L, 0L, 1L, 1L) + ) +}) + +test_that("raises only the spans between first_stop and last_stop", { + # the times before first_stop don't change, and the times at and after + # last_stop are shifted earlier + + result <- tester(trip_id = "t3", first_stop = "s2", last_stop = "s4") + saved <- 600L - max_one + + expect_identical( + secs(times_of(result, "t3")$arrival_time), + secs(slow$arr) - c(0L, 0L, saved, 2L * saved, 2L * saved) + ) +}) + +test_that("unit argument works correctly", { + expect_identical( + tester(min_speed = 10, unit = "m/s"), + tester(min_speed = 36) + ) +}) + +test_that("leaves spans of length 0 unchanged", { + feed <- synthetic_feed( + list(slow), + route_id = "r1", + stop_id = c("s1", "s2", "s2", "s3", "s4") + ) + result <- tester(feed) + arr <- secs(result$stop_times$arrival_time) + + expect_identical(diff(arr), c(max_one, 600L, max_one, max_one)) +}) + +test_that("may result in spans with zero duration", { + result <- tester(trip_id = "t3", min_speed = 1e6) + expect_identical( + times_of(result, "t3")$arrival_time, + rep("08:00:00", 5) + ) +}) + +test_that("leaves spans that end before they start unchanged, with a warning", { + feed <- synthetic_feed( + list( + same_times(c("08:00:00", "08:10:00", "07:00:00", "07:10:00", "07:20:00")) + ), + route_id = "r1" + ) + + w <- tryCatch(tester(feed), warning = function(w) w) + expect_s3_class(w, "gtfstools_backwards_times") + expect_match(conditionMessage(w), "not raised") + + result <- suppressWarnings(tester(feed)) + saved <- 600L - max_one + + expect_identical( + secs(result$stop_times$arrival_time), + secs(feed$stop_times$arrival_time) - + c(0L, saved, saved, 2L * saved, 3L * saved) + ) +}) + +test_that("raises an error if times would be earlier than 00:00:00", { + # the span after last_stop ends before it starts, so it's not reported, but + # it's shifted + + feed <- synthetic_feed( + list( + same_times(c("08:00:00", "08:10:00", "08:20:00", "00:05:00", "00:15:00")) + ), + route_id = "r1" + ) + + expect_no_warning( + e <- tryCatch(tester(feed, last_stop = "s3"), error = function(e) e) + ) + expect_s3_class(e, "gtfstools_time_out_of_range") + expect_identical(conditionCall(e)[[1]], quote(set_min_speed)) +}) From 31aef57f5da913a50052a51914ddafa0be139739 Mon Sep 17 00:00:00 2001 From: rafapereirabr Date: Sun, 1 Nov 2026 14:02:47 -0300 Subject: [PATCH 3/6] fix_gtfs() gains overwrite_stop_times; use shift_trip_end consistently fix_gtfs() passes overwrite_stop_times (default FALSE, last argument so positional calls keep working) to interpolate_stop_times(overwrite = ). set_dwell_time()'s default strategy is "shift_trip_end": the docs, NEWS and tests still said "push_trip_end". Co-Authored-By: Claude Opus 5.5 --- NEWS.md | 3 +- R/fix_gtfs.R | 23 +++++++++++--- R/interpolate_stop_times.R | 7 +++-- man/fix_gtfs.Rd | 16 ++++++++-- man/interpolate_stop_times.Rd | 7 +++-- man/set_dwell_time.Rd | 46 ++++------------------------ tests/testthat/test-fix_gtfs.R | 38 +++++++++++++++++++++++ tests/testthat/test-set_dwell_time.R | 4 +-- 8 files changed, 89 insertions(+), 55 deletions(-) diff --git a/NEWS.md b/NEWS.md index 76d3b54d..0b47423d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,7 +6,8 @@ ## New features -- `set_dwell_time()` gains `strategy`. The default, `"push_trip_end"`, shifts the later times of the trips, as before. `"keep_total_duration"` splits the change around each intermediate stop, keeping the trip durations. +- `fix_gtfs()` gains `overwrite_stop_times`. With `overwrite_stop_times = TRUE`, the times of every stop between the first and the last stop of each trip are replaced by interpolated ones, keeping their dwell times, instead of only filling blank times (see `interpolate_stop_times()`). +- `set_dwell_time()` gains `strategy`. The default, `"shift_trip_end"`, shifts the later times of the trips, as before. `"keep_total_duration"` splits the change around each intermediate stop, keeping the trip durations. - `interpolate_stop_times()` gains `overwrite` and `keep_dwell_times`. With `overwrite = TRUE`, the times of every stop between the first and the last stop of each trip are replaced by interpolated ones, instead of only the blank ones. By default (`keep_dwell_times = TRUE`), these stops keep their original dwell times. ## Bug fixes diff --git a/R/fix_gtfs.R b/R/fix_gtfs.R index 44f5c027..f799ea5c 100644 --- a/R/fix_gtfs.R +++ b/R/fix_gtfs.R @@ -18,6 +18,12 @@ #' @param dwell_time_unit A string representing the time unit in which #' `min_dwell_time` is given. One of `"s"` (seconds, the default), `"min"` #' (minutes), `"h"` (hours) or `"d"` (days). +#' @param overwrite_stop_times A logical. If `FALSE` (the default), only blank +#' stop times are filled. If `TRUE`, the times of every stop between the +#' first and the last stop of each trip are replaced by times interpolated +#' over the distance travelled, keeping their dwell times (see the +#' `overwrite` and `keep_dwell_times` arguments of +#' [interpolate_stop_times()]). #' #' @return The fixed GTFS object. #' @@ -38,7 +44,11 @@ #' stops around them. Speeds are capped before interpolating, so the filled #' times follow the capped times. Interpolated times are rounded to the #' second. Stops whose times can't be interpolated are left blank, with a -#' warning; +#' warning. With `overwrite_stop_times = TRUE`, the times of every +#' intermediate stop are replaced instead, from each trip's first and last +#' times, keeping the stops' dwell times. The trips' durations still +#' follow the capped times, and their travel time is spread evenly over +#' the distance; #' 5. [set_min_dwell_time()], only if `min_dwell_time` is given, delays the #' departure from each intermediate stop whose dwell time is shorter than #' `min_dwell_time`, so that vehicles stay there for at least that long. @@ -100,7 +110,8 @@ fix_gtfs <- function( max_speed = NULL, speed_unit = "km/h", min_dwell_time = NULL, - dwell_time_unit = "s" + dwell_time_unit = "s", + overwrite_stop_times = FALSE ) { gtfs <- assert_and_assign_gtfs_object(gtfs) checkmate::assert_number( @@ -128,6 +139,7 @@ fix_gtfs <- function( upper = max_time_secs / dwell_unit_factors[[dwell_time_unit]], null.ok = TRUE ) + checkmate::assert_logical(overwrite_stop_times, len = 1, any.missing = FALSE) # remove duplicates first, so the other steps don't see repeated rows @@ -148,9 +160,12 @@ fix_gtfs <- function( ) } - # interpolate stop_times + # fill blank stop times, or overwrite the times of intermediate stops - gtfs_fixed <- interpolate_stop_times(gtfs_fixed) + gtfs_fixed <- interpolate_stop_times( + gtfs = gtfs_fixed, + overwrite = overwrite_stop_times + ) # set a minimum dwell time at intermediate stops. this is done after # interpolating, so the filled stops also get it. 'stop_times' is not the diff --git a/R/interpolate_stop_times.R b/R/interpolate_stop_times.R index c5d737a1..65ef33de 100644 --- a/R/interpolate_stop_times.R +++ b/R/interpolate_stop_times.R @@ -1,8 +1,9 @@ #' Interpolate missing stop times #' -#' Fills blank `arrival_time`s and `departure_time`s in `stop_times`, assuming -#' that vehicles travel at a constant speed between consecutive stops with -#' known times (timepoints). +#' Fills blank `arrival_time`s and `departure_time`s in `stop_times`, or +#' overwrites those of the intermediate stops of each trip, assuming that +#' vehicles travel at a constant speed between consecutive stops with known +#' times (timepoints). #' #' @template gtfs #' @param trip_id A character vector including the `trip_id`s to have their diff --git a/man/fix_gtfs.Rd b/man/fix_gtfs.Rd index 3ce0c9eb..3da5ba28 100644 --- a/man/fix_gtfs.Rd +++ b/man/fix_gtfs.Rd @@ -9,7 +9,8 @@ fix_gtfs( max_speed = NULL, speed_unit = "km/h", min_dwell_time = NULL, - dwell_time_unit = "s" + dwell_time_unit = "s", + overwrite_stop_times = FALSE ) } \arguments{ @@ -29,6 +30,13 @@ open to serve passengers, in the unit given by \code{dwell_time_unit}. If \item{dwell_time_unit}{A string representing the time unit in which \code{min_dwell_time} is given. One of \code{"s"} (seconds, the default), \code{"min"} (minutes), \code{"h"} (hours) or \code{"d"} (days).} + +\item{overwrite_stop_times}{A logical. If \code{FALSE} (the default), only blank +stop times are filled. If \code{TRUE}, the times of every stop between the +first and the last stop of each trip are replaced by times interpolated +over the distance travelled, keeping their dwell times (see the +\code{overwrite} and \code{keep_dwell_times} arguments of +\code{\link[=interpolate_stop_times]{interpolate_stop_times()}}).} } \value{ The fixed GTFS object. @@ -58,7 +66,11 @@ a warning; stops around them. Speeds are capped before interpolating, so the filled times follow the capped times. Interpolated times are rounded to the second. Stops whose times can't be interpolated are left blank, with a -warning; +warning. With \code{overwrite_stop_times = TRUE}, the times of every +intermediate stop are replaced instead, from each trip's first and last +times, keeping the stops' dwell times. The trips' durations still +follow the capped times, and their travel time is spread evenly over +the distance; \item \code{\link[=set_min_dwell_time]{set_min_dwell_time()}}, only if \code{min_dwell_time} is given, delays the departure from each intermediate stop whose dwell time is shorter than \code{min_dwell_time}, so that vehicles stay there for at least that long. diff --git a/man/interpolate_stop_times.Rd b/man/interpolate_stop_times.Rd index 63a7f094..e9031f0b 100644 --- a/man/interpolate_stop_times.Rd +++ b/man/interpolate_stop_times.Rd @@ -50,9 +50,10 @@ The order of the rows and the other columns are kept. The given GTFS object is not modified. } \description{ -Fills blank \code{arrival_time}s and \code{departure_time}s in \code{stop_times}, assuming -that vehicles travel at a constant speed between consecutive stops with -known times (timepoints). +Fills blank \code{arrival_time}s and \code{departure_time}s in \code{stop_times}, or +overwrites those of the intermediate stops of each trip, assuming that +vehicles travel at a constant speed between consecutive stops with known +times (timepoints). } \section{Details}{ diff --git a/man/set_dwell_time.Rd b/man/set_dwell_time.Rd index a674aff8..67aedbe5 100644 --- a/man/set_dwell_time.Rd +++ b/man/set_dwell_time.Rd @@ -8,7 +8,7 @@ set_dwell_time( gtfs, dwell_time, unit = "s", - strategy = "push_trip_end", + strategy = "shift_trip_end", trip_id = NULL, stop_id = NULL, from = NULL, @@ -32,7 +32,7 @@ given. One of \code{"s"} (seconds, the default), \code{"min"} (minutes), \code{" \item{strategy}{A string, how the trips absorb the changes in dwell time. One of: \itemize{ -\item \code{"push_trip_end"} (the default): keeps the running times between +\item \code{"shift_trip_end"} (the default): keeps the running times between stops and shifts all the later times of the trip, so the arrival at the last stop moves later (or earlier, if dwell times are shortened). It preserves the implicit average speed between stops but changes the @@ -86,29 +86,10 @@ change. } \section{Details}{ -The following paragraphs describe the default, -\code{strategy = "push_trip_end"}. See the last paragraph for -\code{strategy = "keep_total_duration"}. - -The dwell time of a visit is its \code{departure_time} minus its \code{arrival_time} -in \code{stop_times}, as returned by \code{\link[=get_dwell_time]{get_dwell_time()}}. It is set at the visits -of every specified trip to every specified stop (not of pairs formed by -their elements): the arrival time is kept and the departure time is set to -the arrival time plus \code{dwell_time}. All the later times of the trip, in -\code{stop_sequence} order, are shifted by the change, and changes at several -visits of the same trip add up. At a trip's last stop, only the departure -time changes. A stop visited more than once by the same trip has its dwell -time set at each visit. - -Blank times are never filled. Selected visits with a blank arrival or -departure time are not changed and don't shift the later times, with a -warning when \code{stop_id} is given; fill them beforehand with -\code{\link[=interpolate_stop_times]{interpolate_stop_times()}}. An error is raised if a shift would result in a -negative time (times not in chronological order) or in a time later than -\code{"9999:59:59"}. \code{stop_sequence} is assumed to be unique within each trip -and not \code{NA}. When both \code{trip_id} and \code{stop_id} are given but none of the -trips visits any of the stops, a warning is raised and the GTFS is returned -unchanged. + +Since the first stop is never changed, the departures of trips listed in +\code{frequencies} are not changed either. Blank arrival or departure times +are never filled. For trips listed in \code{frequencies}, the times in \code{stop_times} are a template (see \code{\link[=frequencies_to_stop_times]{frequencies_to_stop_times()}}), so setting the dwell time at the first @@ -129,21 +110,6 @@ never stations: to set the dwell times at a station, pass its children that appear in \code{stop_times} (see \code{\link[=get_children_stops]{get_children_stops()}}). Existing \verb{_secs} columns in \code{stop_times} (e.g. created with \code{\link[=convert_time_to_seconds]{convert_time_to_seconds()}}) are updated to match. - -With \code{strategy = "keep_total_duration"}, visits are selected in the same -way, but the change in dwell time is split around each visit: when the -dwell time is lengthened, its arrival is moved earlier by half of the change -(rounded toward zero, so an odd second goes to the departure) and its -departure later by the rest, and the other way around when it's shortened. -No other time of the trip changes, so the running times to and from the -visit absorb the change (the vehicle runs faster when dwell times are -lengthened) and the trip keeps its duration. The first and last visits with -both an arrival and a departure time of each trip are never changed, as in -\code{\link[=set_min_dwell_time]{set_min_dwell_time()}}. An error is raised if a running time would become -negative, i.e. if the dwell times set take more time than the trip spends -running between the stops (running times that are already negative are not -checked). Since the first stop is never changed, the departures of trips -listed in \code{frequencies} are not changed either. } \examples{ diff --git a/tests/testthat/test-fix_gtfs.R b/tests/testthat/test-fix_gtfs.R index a46cab0e..02207716 100644 --- a/tests/testthat/test-fix_gtfs.R +++ b/tests/testthat/test-fix_gtfs.R @@ -59,6 +59,14 @@ test_that("raises error due to incorrect input types", { tester(min_dwell_time = 30, dwell_time_unit = "sec"), regexp = "dwell_time_unit" ) + expect_error( + tester(overwrite_stop_times = "TRUE"), + regexp = "overwrite_stop_times" + ) + expect_error( + tester(overwrite_stop_times = NA), + regexp = "overwrite_stop_times" + ) }) test_that("raises errors if gtfs doesn't have required tables", { @@ -85,6 +93,9 @@ test_that("results in a dt_gtfs object and doesn't change given gtfs", { expect_s3_class(result, "dt_gtfs") expect_equal(original_gtfs, gtfs, ignore_attr = TRUE) + result <- tester(gtfs, overwrite_stop_times = TRUE) + expect_equal(original_gtfs, gtfs, ignore_attr = TRUE) + # spo has neither duplicated stop times nor blank times, so its dwell times # are set in the table returned by remove_duplicates(), not in a copy made # by a later step @@ -144,6 +155,33 @@ test_that("doesn't cap speeds if max_speed is NULL", { expect_false(anyNA(secs(times$arrival_time))) }) +test_that("overwrite_stop_times replaces the times of intermediate stops", { + # s3 arrives at 08:00:50 and stays 5 seconds + gtfs <- hand_feed() + gtfs$stop_times[ + 4, + `:=`(arrival_time = "08:00:50", departure_time = "08:00:55") + ] + + result <- tester(gtfs, max_speed = NULL) + expect_identical( + result$stop_times$arrival_time, + c("08:00:00", "08:00:25", "08:00:50", "08:00:57", "08:00:58", "08:01:00") + ) + + # the 55 seconds that the trip isn't stopped are spread evenly over its 5 + # segments of equal length + result <- tester(gtfs, max_speed = NULL, overwrite_stop_times = TRUE) + expect_identical( + result$stop_times$arrival_time, + c("08:00:00", "08:00:11", "08:00:22", "08:00:38", "08:00:49", "08:01:00") + ) + expect_identical( + result$stop_times$departure_time, + c("08:00:00", "08:00:11", "08:00:27", "08:00:38", "08:00:49", "08:01:00") + ) +}) + test_that("sets the minimum dwell time at stops with interpolated times", { without_min <- tester() result <- tester(min_dwell_time = 10) diff --git a/tests/testthat/test-set_dwell_time.R b/tests/testthat/test-set_dwell_time.R index cb3a3011..55633b10 100644 --- a/tests/testthat/test-set_dwell_time.R +++ b/tests/testthat/test-set_dwell_time.R @@ -11,7 +11,7 @@ tester <- function(gtfs = get("gtfs", envir = parent.frame()), unit = "s", from = NULL, to = NULL, - strategy = "push_trip_end", + strategy = "shift_trip_end", by_reference = FALSE) { set_dwell_time( gtfs, @@ -107,7 +107,7 @@ test_that("raises errors due to incorrect input types/value", { expect_error(tester(by_reference = NA)) expect_error(tester(by_reference = c(TRUE, TRUE))) expect_error(tester(strategy = "x")) - expect_error(tester(strategy = c("push_trip_end", "keep_total_duration"))) + expect_error(tester(strategy = c("shift_trip_end", "keep_total_duration"))) expect_error(tester(strategy = NA)) # dwell_time is the second argument, so it can be given by position From 95606f555b89503dee2fcefd75a9479fdb11cf32 Mon Sep 17 00:00:00 2001 From: rafapereirabr Date: Sun, 1 Nov 2026 14:03:20 -0300 Subject: [PATCH 4/6] Fix times after midnight written as early morning times Many feeds write times after midnight as "00:30:00" instead of "24:30:00", so trips that cross midnight seem to go back in time and durations, speeds, time-of-day selections and interpolation come out wrong. New fix_overnight_times() rewrites these times to the GTFS form (a step back of more than 12 hours is taken as midnight; frequencies end_times earlier than start_times are moved a day later), and fix_gtfs() runs it first. The functions that calculate or change durations and speeds, interpolate_stop_times(), build_graph() and the time-of-day functions (filter_by_time_of_day(), get_route_frequency(), set_route_frequency(), get_stop_frequency(), get_vkt(), get_vmt(), get_trip_length() segment windows, get_summary()) fix them silently before using them. Each function checks its own feed, so nested calls find the times already fixed and don't change or copy them again. Internal helper has_overnight_trips() reports whether a feed needs the fix. Co-Authored-By: Claude Opus 5.5 --- NAMESPACE | 1 + NEWS.md | 3 + R/build_graph.R | 14 +- R/filter_by_time_of_day.R | 12 + R/fix_gtfs.R | 38 +- R/fix_overnight_times.R | 460 ++++++++++++++++ R/get_dwell_time.R | 11 +- R/get_route_frequency.R | 11 + R/get_stop_frequency.R | 18 +- R/get_summary.R | 10 + R/get_trip_duration.R | 11 + R/get_trip_length.R | 12 + R/get_trip_segment_duration.R | 9 + R/get_trip_speed.R | 14 +- R/get_vkt.R | 4 +- R/get_vmt.R | 3 +- R/interpolate_stop_times.R | 23 +- R/set_dwell_time.R | 24 +- R/set_max_speed.R | 21 +- R/set_min_dwell_time.R | 4 +- R/set_route_frequency.R | 10 + R/set_trip_speed.R | 14 +- _pkgdown.yml | 1 + man/build_graph.Rd | 5 +- man/filter_by_time_of_day.Rd | 6 + man/fix_gtfs.Rd | 25 +- man/fix_overnight_frequencies.Rd | 19 + man/fix_overnight_in_call.Rd | 34 ++ man/fix_overnight_stop_times.Rd | 42 ++ man/fix_overnight_times.Rd | 88 ++++ man/get_dwell_time.Rd | 6 +- man/get_route_frequency.Rd | 6 + man/get_stop_frequency.Rd | 4 + man/get_summary.Rd | 5 + man/get_trip_duration.Rd | 5 + man/get_trip_length.Rd | 3 + man/get_trip_segment_duration.Rd | 4 + man/get_trip_speed.Rd | 6 +- man/get_vkt.Rd | 4 +- man/get_vmt.Rd | 3 +- man/has_overnight_trips.Rd | 25 + man/interpolate_stop_times.Rd | 8 +- man/set_dwell_time.Rd | 17 +- man/set_max_speed.Rd | 12 +- man/set_min_dwell_time.Rd | 4 +- man/set_route_frequency.Rd | 4 + man/set_trip_speed.Rd | 7 +- tests/testthat/setup-overnight_feed.R | 38 ++ tests/testthat/test-build_graph.R | 41 +- tests/testthat/test-filter_by_time_of_day.R | 18 + tests/testthat/test-fix_gtfs.R | 34 +- tests/testthat/test-fix_overnight_times.R | 489 ++++++++++++++++++ tests/testthat/test-get_dwell_time.R | 35 ++ tests/testthat/test-get_route_frequency.R | 36 ++ tests/testthat/test-get_stop_frequency.R | 30 ++ tests/testthat/test-get_trip_duration.R | 54 +- tests/testthat/test-get_trip_length.R | 29 ++ .../testthat/test-get_trip_segment_duration.R | 30 ++ tests/testthat/test-get_trip_speed.R | 12 + tests/testthat/test-interpolate_stop_times.R | 51 +- tests/testthat/test-set_dwell_time.R | 55 +- tests/testthat/test-set_max_speed.R | 49 +- tests/testthat/test-set_min_dwell_time.R | 36 +- tests/testthat/test-set_route_frequency.R | 25 + tests/testthat/test-set_trip_speed.R | 32 +- vignettes/editing.Rmd | 13 +- vignettes/graphs.Rmd | 3 - 67 files changed, 2023 insertions(+), 157 deletions(-) create mode 100644 R/fix_overnight_times.R create mode 100644 man/fix_overnight_frequencies.Rd create mode 100644 man/fix_overnight_in_call.Rd create mode 100644 man/fix_overnight_stop_times.Rd create mode 100644 man/fix_overnight_times.Rd create mode 100644 man/has_overnight_trips.Rd create mode 100644 tests/testthat/setup-overnight_feed.R create mode 100644 tests/testthat/test-fix_overnight_times.R diff --git a/NAMESPACE b/NAMESPACE index 6d360b36..cb5c1e38 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -28,6 +28,7 @@ export(filter_by_time_of_day) export(filter_by_trip_id) export(filter_by_weekday) export(fix_gtfs) +export(fix_overnight_times) export(frequencies_to_stop_times) export(get_active_services) export(get_calendar_overlap) diff --git a/NEWS.md b/NEWS.md index fc55d4e3..05b09a15 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,9 @@ ## New features +- New function `fix_overnight_times()` rewrites the times of trips that run past midnight but are written as early morning times (e.g. `"00:30:00"` instead of `"24:30:00"`), as required by the GTFS specification. `fix_gtfs()` now runs it before capping speeds and filling blank stop times, so these trips are no longer left with blank times. +- The functions that calculate or change durations and speeds (`get_trip_duration()`, `get_trip_segment_duration()`, `get_trip_speed()`, `get_route_speed()`, `get_dwell_time()`, `set_trip_speed()`, `set_route_speed()`, `set_max_speed()`, `set_dwell_time()` and `set_min_dwell_time()`), `interpolate_stop_times()` and `build_graph()` now fix these times the same way before using them, so they give correct results for trips that run past midnight. The functions that return a GTFS object return it with the times fixed. Their `from` and `to` arguments are compared with the fixed times, so a visit written as `"00:30:00"` after midnight is now matched by a time of day such as `"24:00:00"`-`"25:00:00"`, not `"00:00:00"`-`"01:00:00"`, and `build_graph()` places these trips at their real times. +- The functions that use a time of day (`filter_by_time_of_day()`, `get_route_frequency()`, `set_route_frequency()`, `get_stop_frequency()`, `get_vkt()`, `get_vmt()` and `get_trip_length()` with `by = "segment"`) and `get_summary()` also fix these times first, as well as `frequencies` entries whose `end_time` is earlier than their `start_time` (e.g. `"23:00:00"`-`"01:00:00"`). `get_route_frequency()` and the functions based on it used to count a trip that runs past midnight at its time written as an early morning time, instead of at its real departure. `filter_by_time_of_day()` and `set_route_frequency()` return the feed with these times fixed. - `interpolate_stop_times()` gains `overwrite` and `keep_dwell_times`. With `overwrite = TRUE`, the times of every stop between the first and the last stop of each trip are replaced by interpolated ones, instead of only the blank ones. By default (`keep_dwell_times = TRUE`), these stops keep their original dwell times. ## Bug fixes diff --git a/R/build_graph.R b/R/build_graph.R index 4010f1d2..a7c5e1d3 100644 --- a/R/build_graph.R +++ b/R/build_graph.R @@ -67,7 +67,10 @@ #' should usually be filtered to a single day first, with [filter_by_date()] #' or [filter_by_weekday()]. Departures after midnight have times past #' "24:00:00", so use a time of day such as "24:00:00"-"26:00:00" for them. -#' Trips whose times wrongly go back to "00:00:00" after midnight are placed +#' Times after midnight written as early morning times (e.g. `"00:30:00"` +#' instead of `"24:30:00"`) are first fixed with [fix_overnight_times()], as +#' are `end_time`s earlier than `start_time`s in `frequencies`, so these trips +#' are placed at their real times. Trips whose times still go back are placed #' by their earliest time. #' #' Within a trip, consecutive `stop_times` entries at the same stop are one @@ -224,6 +227,13 @@ build_graph <- function(gtfs, gtfsio::assert_field_class(gtfs, "stops", "stop_name", "character") } + # times after midnight written as early morning times are fixed first, so + # trips are placed at their real times. the departures of trips listed in + # 'frequencies' come from its start and end times, which are fixed too + # (fields of the wrong class are reported later, when they are used) + + gtfs <- fix_overnight_in_call(gtfs, frequencies = TRUE) + network <- build_service_network( gtfs, from_secs, @@ -510,7 +520,7 @@ build_service_network <- function(gtfs, # in-vehicle links connect the service nodes of consecutive visits of each # trip, whose rows are contiguous. times that are missing or that decrease - # (e.g. times after midnight written as 00:xx) are not used + # (by 12 hours or less, since the others were fixed) are not used has_next <- c(trip_run[-1L] == trip_run[-length(trip_run)], FALSE) current <- which(has_next) diff --git a/R/filter_by_time_of_day.R b/R/filter_by_time_of_day.R index 8a02b051..69ecc8a6 100644 --- a/R/filter_by_time_of_day.R +++ b/R/filter_by_time_of_day.R @@ -26,6 +26,12 @@ #' @section Details: #' The time of day includes both `from` and `to`. #' +#' Times after midnight written as early morning times (e.g. `"00:30:00"` +#' instead of `"24:30:00"`), and `end_time`s earlier than `start_time`s in +#' `frequencies`, are first fixed with [fix_overnight_times()], so these +#' times are compared with the time of day as they should be, and the +#' returned GTFS has them fixed. +#' #' Trips listed in `frequencies` are filtered through their `frequencies` #' entries; their `stop_times` entries are templates and are kept whatever #' their times. With `keep = TRUE`, the entries that overlap the time of day @@ -132,6 +138,12 @@ filter_by_time_of_day <- function(gtfs, to_secs <- string_to_seconds(to) if (from_secs > to_secs) stop("'from' must be lower than 'to'.") + # times after midnight written as early morning times are fixed first, so + # they are compared with the time of day as they should be. the returned + # feed keeps them fixed + + gtfs <- fix_overnight_in_call(gtfs, frequencies = TRUE) + # if the gtfs contains a frequencies table, it's important to keep the # stop_times of the trips listed in there intact. # this is because their stop_times are only templates, and the departure and diff --git a/R/fix_gtfs.R b/R/fix_gtfs.R index 44f5c027..fc84bb8b 100644 --- a/R/fix_gtfs.R +++ b/R/fix_gtfs.R @@ -1,10 +1,10 @@ #' Fix common problems in GTFS feeds #' #' Cleans a GTFS feed in one call, by removing duplicated entries and unused -#' ids, optionally capping the speed of trips, filling blank stop times, -#' optionally setting a minimum dwell time at intermediate stops and -#' recalculating the distances travelled along shapes (replacing any existing -#' ones). +#' ids, fixing times after midnight written as early morning times, optionally +#' capping the speed of trips, filling blank stop times, optionally setting a +#' minimum dwell time at intermediate stops and recalculating the distances +#' travelled along shapes (replacing any existing ones). #' #' @template gtfs #' @param max_speed A number. The maximum speed of trips, in the unit given by @@ -29,17 +29,21 @@ #' entries of every table that no remaining trip uses (see its #' documentation for the exceptions). Note that this removes data from the #' feed; -#' 3. [set_max_speed()], only if `max_speed` is given, slows down the parts of +#' 3. [fix_overnight_times()] rewrites the times after midnight written as +#' early morning times (e.g. `"00:30:00"` instead of `"24:30:00"`), so the +#' next steps work on these trips too. Only times that go back by more +#' than 12 hours are taken as times after midnight, and the times that +#' still go back are left unchanged, with a warning; +#' 4. [set_max_speed()], only if `max_speed` is given, slows down the parts of #' trips that are faster than `max_speed`. Only spans between stops with -#' known times are capped, and spans that end before they start (e.g. -#' times after midnight written as `"00:30:00"`) are left unchanged, with -#' a warning; -#' 4. [interpolate_stop_times()] fills blank stop times from the times of the +#' known times are capped, and spans that still end before they start are +#' left unchanged, with a warning; +#' 5. [interpolate_stop_times()] fills blank stop times from the times of the #' stops around them. Speeds are capped before interpolating, so the filled #' times follow the capped times. Interpolated times are rounded to the #' second. Stops whose times can't be interpolated are left blank, with a #' warning; -#' 5. [set_min_dwell_time()], only if `min_dwell_time` is given, delays the +#' 6. [set_min_dwell_time()], only if `min_dwell_time` is given, delays the #' departure from each intermediate stop whose dwell time is shorter than #' `min_dwell_time`, so that vehicles stay there for at least that long. #' The minimum is rounded to the second. The later times of each changed @@ -48,10 +52,8 @@ #' filling blank times, so the filled stops also get the minimum. Only the #' stops between the first and the last stop with times of each trip get #' the minimum, so trips keep their start times. Stops with negative dwell -#' times (departures earlier than arrivals) also get it, see -#' [set_min_dwell_time()] for times after midnight written as -#' `"00:30:00"`; -#' 6. [interpolate_shape_dist_traveled()], only if the feed has a `shapes` +#' times (departures earlier than arrivals) also get it; +#' 7. [interpolate_shape_dist_traveled()], only if the feed has a `shapes` #' table and `trips` has a `shape_id` column, replaces the #' `shape_dist_traveled` columns of `stop_times` and `shapes` with the #' distances, in meters, from the start of each shape. Existing values, @@ -64,7 +66,8 @@ #' above, so some warnings (e.g. about trips not linked to a shape) may be #' raised more than once. #' -#' @seealso [remove_duplicates()], [remove_unused_ids()], [set_max_speed()], +#' @seealso [remove_duplicates()], [remove_unused_ids()], +#' [fix_overnight_times()], [set_max_speed()], #' [interpolate_stop_times()], [set_min_dwell_time()], #' [interpolate_shape_dist_traveled()] #' @@ -137,6 +140,11 @@ fix_gtfs <- function( gtfs_fixed <- remove_unused_ids(gtfs_fixed) + # fix times after midnight before the steps that use the times, which skip + # the spans that go back + + gtfs_fixed <- fix_overnight_times(gtfs_fixed) + # cap max speed. this is done before interpolating, so the interpolated # times follow the capped times diff --git a/R/fix_overnight_times.R b/R/fix_overnight_times.R new file mode 100644 index 00000000..0b842d7a --- /dev/null +++ b/R/fix_overnight_times.R @@ -0,0 +1,460 @@ +#' Fix times after midnight written as early morning times +#' +#' Rewrites the times after midnight of trips that run past it, when they are +#' written as early morning times (e.g. `"00:10:00"` after `"23:50:00"`), as +#' times past `"24:00:00"` (e.g. `"24:10:00"`), as required by the GTFS +#' specification. All gtfstools functions that calculate or edit durations +#' and speeds or use information on time of day use `fix_overnight_times()` +#' internally to fix the overnight times. +#' +#' @template gtfs +#' +#' @return A GTFS object with the time columns of its `stop_times` and the +#' `end_time` column of its `frequencies` fixed. +#' +#' @section Details: +#' The arrival and departure times of each trip are read in order of +#' `stop_sequence`, ignoring blank times. A time more than 12 hours earlier +#' than the previous time of its trip is taken as a time after midnight, so +#' it and the later times of the trip are moved one day later (e.g. +#' `"00:10:00"` becomes `"24:10:00"`). Trips that run past a second midnight +#' are moved two days, and so on. A time more than 12 hours later than the +#' previous time of its trip undoes one of these days, so feeds that mix both +#' forms are also fixed (e.g. `"24:30:00"`, `"00:40:00"`, `"24:50:00"` +#' becomes `"24:30:00"`, `"24:40:00"`, `"24:50:00"`), and trips whose times +#' already follow the specification are left unchanged. Blank times are kept +#' blank. Only `stop_times` and `frequencies` are modified; their existing +#' `_secs` columns are updated to match. +#' +#' Times earlier than the previous time of their trip by 12 hours or less are +#' left unchanged, with a warning. Times are never moved earlier, so trips +#' with known times more than 12 hours apart that follow the specification +#' (e.g. `"06:00:00"`, `"19:30:00"`) are left unchanged. Rows with a missing +#' `trip_id` or `stop_sequence` are left unchanged, and stops with duplicated +#' `stop_sequence`s are read in the order of their rows (see +#' [remove_duplicates()]). +#' +#' Trips whose times are all written after midnight as early morning times +#' (e.g. a trip of the previous service day departing at `"00:30:00"`) can't +#' be told apart from trips of the service day and are left unchanged. +#' +#' In `frequencies`, an `end_time` earlier than the `start_time` of its row +#' is moved by whole days to the first time later than the `start_time` +#' (e.g. `"23:00:00"`-`"01:00:00"` becomes `"23:00:00"`-`"25:00:00"`). A +#' `start_time` written as an early morning time is left unchanged. The +#' template times of trips listed in `frequencies` are fixed as any other +#' times, which doesn't change their departures, since only the differences +#' between them are used. +#' +#' An error is raised if a fixed time would be later than `"9999:59:59"`. +#' +#' +#' @seealso [fix_gtfs()], [convert_time_to_seconds()] +#' +#' @examplesIf requireNamespace("dplyr", quietly = TRUE) +#' \dontshow{ +#' old_dt_threads <- data.table::setDTthreads(1) +#' } +#' library(dplyr) +#' +#' data_path <- system.file("extdata/poa_gtfs.zip", package = "gtfstools") +#' +#' gtfs <- read_gtfs(data_path) +#' +#' # this trip runs past midnight, but its last time is written as "00:02:00" +#' gtfs$stop_times |> +#' filter(trip_id == "T2-1@1#2310", arrival_time != "") |> +#' select(trip_id, stop_sequence, arrival_time, departure_time) +#' +#' fixed_gtfs <- fix_overnight_times(gtfs) +#' +#' fixed_gtfs$stop_times |> +#' filter(trip_id == "T2-1@1#2310", arrival_time != "") |> +#' select(trip_id, stop_sequence, arrival_time, departure_time) +#' \dontshow{data.table::setDTthreads(old_dt_threads)} +#' +#' @export +fix_overnight_times <- function(gtfs) { + gtfs <- assert_and_assign_gtfs_object(gtfs) + + gtfsio::assert_field_class( + gtfs, + "stop_times", + c("trip_id", "stop_sequence", "arrival_time", "departure_time"), + c("character", "integer", "character", "character") + ) + + if (gtfsio::check_file_exists(gtfs, "frequencies")) { + gtfsio::assert_field_class( + gtfs, + "frequencies", + c("start_time", "end_time"), + c("character", "character") + ) + } + + gtfs <- fix_overnight_stop_times(gtfs) + gtfs <- fix_overnight_frequencies(gtfs) + + return(gtfs) +} + + +#' Fix the times after midnight of `stop_times` +#' +#' Also used by the functions that calculate speeds and durations, which fix +#' the times silently before using them. +#' +#' @param gtfs A GTFS object whose `stop_times` has the `trip_id`, +#' `stop_sequence`, `arrival_time` and `departure_time` fields, of the +#' classes asserted by [fix_overnight_times()]. +#' @param trip_id A character vector with the `trip_id`s to fix, or `NULL` +#' (the default) for every trip. Each trip is fixed on its own, so the +#' functions that only use some trips fix only these. +#' @param warn Whether to warn about the times that still go back after the +#' fix, and about malformed times. +#' @param by_reference Whether to write the fixed times into +#' `gtfs$stop_times` itself. If `FALSE`, the table is copied before being +#' changed. +#' @param detect_only Whether to only detect the times to fix, without fixing +#' them. +#' +#' @return The GTFS object, or, with `detect_only = TRUE`, `TRUE` if any time +#' would be fixed and `FALSE` otherwise. +#' +#' @keywords internal +fix_overnight_stop_times <- function( + gtfs, + trip_id = NULL, + warn = TRUE, + by_reference = FALSE, + detect_only = FALSE +) { + stop_times <- gtfs$stop_times + + # rows with a missing trip_id or stop_sequence can't be placed in a trip, + # so they are left alone + + is_candidate <- !is.na(stop_times$trip_id) & !is.na(stop_times$stop_sequence) + if (!is.null(trip_id)) { + is_candidate <- is_candidate & stop_times$trip_id %chin% trip_id + } + + # the rows are sorted so that the rows of each trip are contiguous. the + # trips are sorted by their order of appearance, not by their ids, because + # the radix sort of order() fails on ids in a native encoding with + # non-ASCII characters (e.g. on Windows) + + rows <- which(is_candidate) + candidate_trips <- stop_times$trip_id[rows] + trip_number <- data.table::chmatch(candidate_trips, unique(candidate_trips)) + ord <- order(trip_number, stop_times$stop_sequence[rows], method = "radix") + rows <- rows[ord] + trip_number <- trip_number[ord] + + # the arrival and departure times of each stop are read in turn, so a wrap + # between them is found too. blank times are skipped + + # only the selected rows are converted, from the existing *_secs columns + # (as in time_secs()) or from the time strings. malformed times are + # reported by the functions that call this one silently, so they are not + # reported twice + + rows_secs <- function(time_col) { + secs_col <- paste0(time_col, "_secs") + if (secs_col %chin% names(stop_times)) { + return(stop_times[[secs_col]][rows]) + } + return(string_to_seconds(stop_times[[time_col]][rows])) + } + + n_rows <- length(rows) + secs <- withCallingHandlers( + as.numeric(rbind(rows_secs("arrival_time"), rows_secs("departure_time"))), + gtfstools_malformed_time = function(cnd) { + if (!warn) invokeRestart("muffleWarning") + } + ) + row <- rep(rows, each = 2L) + is_arrival <- rep(c(TRUE, FALSE), times = n_rows) + + # trips are told apart by the numbers they were sorted by, which is cheaper + # than comparing their ids on the interleaved times + + trip_run <- rep(trip_number, each = 2L) + + is_known <- !is.na(secs) + secs <- secs[is_known] + row <- row[is_known] + is_arrival <- is_arrival[is_known] + trip_run <- trip_run[is_known] + + n <- length(secs) + if (n == 0) { + return(if (detect_only) FALSE else gtfs) + } + + is_first <- c(TRUE, trip_run[-1L] != trip_run[-n]) + trip_idx <- cumsum(is_first) + + # a step back of more than 12 hours between consecutive times of a trip + # crosses midnight, so the times after it are one day later than written. + # a step forward of more than 12 hours undoes one of these days, so feeds + # that mix "24:xx" and "00:xx" times are fixed too. round() rounds a step of + # exactly 12 hours to 0 + + step <- c(0, diff(secs)) + step[is_first] <- 0 + days <- cumsum(-round(step / 86400)) + + # the days are counted from the start of each trip, and kept from going + # below 0 by subtracting their running minimum within the trip, so times are + # never moved earlier and long trips that follow the spec (e.g. "06:00:00", + # "19:30:00") are left unchanged. each trip is shifted below all the + # previous ones, so a single cummin() restarts at the start of each trip + + days <- days - days[is_first][trip_idx] + trip_shift <- trip_idx * (2 * max(abs(days)) + 1) + days <- days - (cummin(days - trip_shift) + trip_shift) + fixed <- secs + days * 86400 + + # steps that still go back are left as they are, with a warning + + is_backwards <- !is_first & c(0, diff(fixed)) < 0 + + if (warn && any(is_backwards)) { + backwards_trips <- unique(stop_times$trip_id[row[is_backwards]]) + abbreviated <- abbreviate_ids(backwards_trips) + cli::cli_warn( + c( + paste0( + "{length(backwards_trips)} trip{?s} {?has/have} times earlier ", + "than the previous time of the trip, which {?was/were} not fixed: ", + "{.val {abbreviated$shown}}{abbreviated$more}." + ), + "i" = paste0( + "Only times that go back by more than 12 hours are taken as times ", + "after midnight." + ) + ), + class = "gtfstools_backwards_times" + ) + } + + is_changed <- days > 0 + if (detect_only) { + return(any(is_changed)) + } + if (!any(is_changed)) { + return(gtfs) + } + + # seconds_to_string() can't write times later than 9999:59:59 + + is_invalid <- is_changed & fixed > max_time_secs + + if (any(is_invalid)) { + invalid_trips <- unique(stop_times$trip_id[row[is_invalid]]) + abbreviated <- abbreviate_ids(invalid_trips) + cli::cli_abort( + paste0( + "Fixing the times after midnight results in times later than ", + "{.val 9999:59:59} in the following ", + "{cli::qty(length(invalid_trips))}trip{?s}: ", + "{.val {abbreviated$shown}}{abbreviated$more}." + ), + class = "gtfstools_time_out_of_range" + ) + } + + if (!by_reference) { + stop_times <- data.table::copy(stop_times) + } + + is_arr_changed <- is_changed & is_arrival + data.table::set( + stop_times, + i = row[is_arr_changed], + j = "arrival_time", + value = seconds_to_string(as.integer(fixed[is_arr_changed])) + ) + + is_dep_changed <- is_changed & !is_arrival + data.table::set( + stop_times, + i = row[is_dep_changed], + j = "departure_time", + value = seconds_to_string(as.integer(fixed[is_dep_changed])) + ) + + # refresh pre-existing *_secs columns, which other functions use as-is + + refresh_secs_columns( + stop_times, + c("arrival_time", "departure_time"), + unique(row[is_changed]) + ) + + gtfs$stop_times <- stop_times + return(gtfs) +} + + +#' Fix the end times after midnight of `frequencies` +#' +#' @param gtfs A GTFS object, whose `frequencies` is copied before being +#' changed. +#' +#' @return The GTFS object. +#' +#' @keywords internal +fix_overnight_frequencies <- function(gtfs) { + frequencies <- gtfs$frequencies + if (is.null(frequencies)) { + return(gtfs) + } + + start <- time_secs(frequencies, "start_time") + end <- time_secs(frequencies, "end_time") + wrapped <- which(end < start) + + if (length(wrapped) == 0) { + return(gtfs) + } + + fixed <- end[wrapped] + + 86400 * (floor((start[wrapped] - end[wrapped]) / 86400) + 1) + + if (any(fixed > max_time_secs)) { + cli::cli_abort( + paste0( + "Fixing the times after midnight results in times later than ", + "{.val 9999:59:59} in {.field frequencies}." + ), + class = "gtfstools_time_out_of_range" + ) + } + + frequencies <- data.table::copy(frequencies) + data.table::set( + frequencies, + i = wrapped, + j = "end_time", + value = seconds_to_string(as.integer(fixed)) + ) + refresh_secs_columns(frequencies, "end_time", wrapped) + + gtfs$frequencies <- frequencies + return(gtfs) +} + + +#' Check whether a feed has trips with times after midnight to fix +#' +#' Detects the trips whose times after midnight are written as early morning +#' times (e.g. `"00:30:00"` instead of `"24:30:00"`), as [fix_overnight_times()] +#' does, without fixing them: trips with a time more than 12 hours earlier than +#' the previous time of the trip, and `frequencies` entries whose `end_time` is +#' earlier than their `start_time`. Trips already written with times past +#' `"24:00:00"` don't count. Tables whose fields can't be checked (e.g. a +#' `stop_times` without an integer `stop_sequence`) are skipped. +#' +#' @param gtfs A GTFS object. +#' +#' @return `TRUE` if [fix_overnight_times()] would change any time, `FALSE` +#' otherwise. +#' +#' @keywords internal +has_overnight_trips <- function(gtfs) { + is_stop_times_valid <- gtfsio::check_field_class( + gtfs, + "stop_times", + c("trip_id", "stop_sequence", "arrival_time", "departure_time"), + c("character", "integer", "character", "character") + ) + # the check is silent: malformed times are reported by the functions that + # use the times + + if (isTRUE(is_stop_times_valid)) { + has_wrapped_times <- fix_overnight_stop_times( + gtfs, + warn = FALSE, + detect_only = TRUE + ) + if (has_wrapped_times) return(TRUE) + } + + is_frequencies_valid <- gtfsio::check_field_class( + gtfs, + "frequencies", + c("start_time", "end_time"), + c("character", "character") + ) + if (isTRUE(is_frequencies_valid)) { + is_wrapped <- withCallingHandlers( + time_secs(gtfs$frequencies, "end_time") < + time_secs(gtfs$frequencies, "start_time"), + gtfstools_malformed_time = function(cnd) invokeRestart("muffleWarning") + ) + return(any(is_wrapped, na.rm = TRUE)) + } + + return(FALSE) +} + + +#' Fix the times after midnight silently +#' +#' Fixes the times after midnight of `stop_times` (and of `frequencies`) +#' silently, as the functions that use the times do before using them. A +#' feed whose times were already fixed is returned unchanged, without being +#' copied, so a function called by another one doesn't fix them again. +#' +#' @param gtfs A GTFS object. +#' @param trip_id A character vector with the `trip_id`s to fix, or `NULL` +#' for every trip. +#' @param frequencies Whether to fix the `end_time`s of `frequencies` too. +#' @param by_reference Whether to write the fixed times into +#' `gtfs$stop_times` itself. +#' +#' @return The GTFS object. +#' +#' @keywords internal +fix_overnight_in_call <- function( + gtfs, + trip_id = NULL, + frequencies = FALSE, + by_reference = FALSE +) { + # tables whose fields can't be fixed are left as they are, and the callers' + # own assertions report them + + is_stop_times_valid <- gtfsio::check_field_class( + gtfs, + "stop_times", + c("trip_id", "stop_sequence", "arrival_time", "departure_time"), + c("character", "integer", "character", "character") + ) + if (isTRUE(is_stop_times_valid)) { + gtfs <- fix_overnight_stop_times( + gtfs, + trip_id, + warn = FALSE, + by_reference = by_reference + ) + } + + if (frequencies) { + is_frequencies_valid <- gtfsio::check_field_class( + gtfs, + "frequencies", + c("start_time", "end_time"), + c("character", "character") + ) + if (isTRUE(is_frequencies_valid)) { + gtfs <- fix_overnight_frequencies(gtfs) + } + } + + return(gtfs) +} diff --git a/R/get_dwell_time.R b/R/get_dwell_time.R index 67f9835d..eaee0972 100644 --- a/R/get_dwell_time.R +++ b/R/get_dwell_time.R @@ -40,8 +40,12 @@ #' [interpolate_stop_times()]. Negative dwell times (departure earlier than #' arrival) are returned as they are. #' +#' Times after midnight written as early morning times (e.g. `"00:30:00"` +#' instead of `"24:30:00"`) are fixed with [fix_overnight_times()] before the +#' dwell times are calculated. +#' #' `from` and `to` are compared with the `arrival_time` of each visit as -#' written in `stop_times` (template times, for trips listed in +#' written in `stop_times`, once fixed (template times, for trips listed in #' `frequencies`), so use times past `"24:00:00"` for arrivals after midnight. #' Visits with a blank arrival time are never included when `from` or `to` is #' given. @@ -101,6 +105,11 @@ get_dwell_time <- function(gtfs, c("character", "character", "integer", "character", "character") ) + # times after midnight written as early morning times are fixed first, so + # they don't go back + + gtfs <- fix_overnight_in_call(gtfs, trip_id) + rows <- select_dwell_time_calls(gtfs$stop_times, trip_id, stop_id) arr <- string_to_seconds(gtfs$stop_times$arrival_time[rows]) diff --git a/R/get_route_frequency.R b/R/get_route_frequency.R index 8b4b175c..7ddd6a94 100644 --- a/R/get_route_frequency.R +++ b/R/get_route_frequency.R @@ -37,6 +37,12 @@ #' (unlike in [filter_by_time_of_day()], which also keeps entries at `to`). #' Departures after midnight are listed with times past "24:00:00", so use a #' time of day such as "24:00:00"-"26:00:00" to count them. +#' Times after midnight written as early morning times (e.g. `"00:30:00"` +#' instead of `"24:30:00"`), and `end_time`s earlier than `start_time`s in +#' `frequencies`, are first fixed with [fix_overnight_times()], so these +#' trips are counted at their real times. Without an integer `stop_sequence` +#' column, the times of `stop_times` can't be ordered and are used as +#' written. #' #' The mean headway is the length of the time of day divided by the number of #' departures, so it overstates the actual headway when service starts or ends @@ -155,6 +161,11 @@ get_trip_departures <- function(gtfs, trip_id) { relevant_trips <- trip_id + # times after midnight written as early morning times are fixed first, so + # each trip departs at its real time. only the relevant trips are fixed + + gtfs <- fix_overnight_in_call(gtfs, relevant_trips, frequencies = TRUE) + # the departures of trips listed in 'frequencies' come from the # 'frequencies' table, while their 'stop_times' entries are just templates diff --git a/R/get_stop_frequency.R b/R/get_stop_frequency.R index 983034d3..f3c3af9e 100644 --- a/R/get_stop_frequency.R +++ b/R/get_stop_frequency.R @@ -56,6 +56,10 @@ #' times are equal to `to`. Departures after midnight are listed with times #' greater than "24:00:00" and belong to the previous service day, so a time #' of day such as "24:00:00"-"26:00:00" should be used to account for them. +#' Times after midnight written as early morning times (e.g. `"00:30:00"` +#' instead of `"24:30:00"`), and `end_time`s earlier than `start_time`s in +#' `frequencies`, are first fixed with [fix_overnight_times()], so these +#' departures are counted at their real times. #' #' The mean headway is the length of the time of day divided by the number of #' departures. It is an average over the whole time of day, so it is larger @@ -148,9 +152,19 @@ get_stop_frequency <- function(gtfs, gtfs$stop_times$stop_id %chin% relevant_stops ] relevant_trips <- unique(relevant_trips) + } else { + relevant_trips <- NULL + } + + # times after midnight written as early morning times are fixed first, so + # the departures are compared with the time of day as they should be. only + # the relevant trips are fixed + + gtfs <- fix_overnight_in_call(gtfs, relevant_trips, frequencies = TRUE) + if (is.null(relevant_trips)) { stop_times <- gtfs$stop_times[ - trip_id %chin% relevant_trips, + , .( trip_id, stop_id, @@ -160,7 +174,7 @@ get_stop_frequency <- function(gtfs, ] } else { stop_times <- gtfs$stop_times[ - , + trip_id %chin% relevant_trips, .( trip_id, stop_id, diff --git a/R/get_summary.R b/R/get_summary.R index 3ef63506..dc31e543 100644 --- a/R/get_summary.R +++ b/R/get_summary.R @@ -89,6 +89,11 @@ #' The report requires the `{ggplot2}`, `{knitr}` and `{rmarkdown}` packages #' and [Pandoc](https://pandoc.org/). #' +#' Times after midnight written as early morning times (e.g. `"00:30:00"` +#' instead of `"24:30:00"`), and `end_time`s earlier than `start_time`s in +#' `frequencies`, are first fixed with [fix_overnight_times()], if +#' `stop_sequence` is an integer column. +#' #' The time-of-day filter keeps whole trips (as [filter_by_time_of_day()] with #' `full_trips = TRUE`), so trip speeds and lengths refer to whole trips. #' @@ -306,6 +311,11 @@ get_summary <- function( cli::cli_progress_update(id = progress_bar, status = "Applying filters") + # times after midnight written as early morning times are fixed once, here, + # so the functions used below find them already fixed + + gtfs <- fix_overnight_in_call(gtfs, frequencies = TRUE) + feed <- gtfs if (!is.null(agency_id)) { # feeds whose routes' agency_id is missing or blank have a single agency diff --git a/R/get_trip_duration.R b/R/get_trip_duration.R index 4ba30e55..53257fbd 100644 --- a/R/get_trip_duration.R +++ b/R/get_trip_duration.R @@ -28,6 +28,11 @@ #' [convert_time_to_seconds()]) are used as-is, not recalculated from the time #' strings. #' +#' Times after midnight written as early morning times (e.g. `"00:30:00"` +#' instead of `"24:30:00"`) are fixed with [fix_overnight_times()] before the +#' durations are calculated, if `stop_sequence` is an integer column (without +#' it, the times can't be ordered and are used as written). +#' #' `from` and `to` are compared with the departure time of the first stop of #' each trip, in `stop_sequence` order (which must then be an integer column). #' The time is read as the durations are: from `departure_time_secs`, if @@ -84,6 +89,12 @@ get_trip_duration <- function(gtfs, gtfsio::assert_field_class(gtfs, "stop_times", "stop_sequence", "integer") } + # times after midnight written as early morning times are fixed first, so + # they don't go back. without an integer stop_sequence the times can't be + # ordered, so they are used as written + + gtfs <- fix_overnight_in_call(gtfs, trip_id) + # select trip_ids to get duration of and raise warning if a given trip_id # doesn't exist in 'stop_times' # also create object with filtered stop_times. an empty selection goes diff --git a/R/get_trip_length.R b/R/get_trip_length.R index 3165b4e5..3f3ccac2 100644 --- a/R/get_trip_length.R +++ b/R/get_trip_length.R @@ -64,6 +64,9 @@ #' `from` and `to` are compared with the `departure_time` of the first stop of #' each trip (in `stop_sequence` order) or segment, as written in #' `stop_times`, so use times past `"24:00:00"` for departures after midnight. +#' With `by = "segment"`, times after midnight written as early morning times +#' (e.g. `"00:30:00"` instead of `"24:30:00"`) are first fixed with +#' [fix_overnight_times()]; a trip's first departure is never changed by it. #' For trips listed in `frequencies`, these are template times, not the times #' of each departure. Trips and segments whose departure is blank are never #' included when `from` or `to` is given. @@ -160,6 +163,15 @@ get_trip_length <- function(gtfs, if (method == "shapes") assert_shapes_fields(gtfs) + # with by = "segment", the segments within the time of day are selected by + # the departure at their first stop, so times after midnight written as + # early morning times are fixed first. a trip's first departure is never + # changed by the fix, so by = "trip" doesn't need it + + if (is_windowed && by == "segment") { + gtfs <- fix_overnight_in_call(gtfs, trip_id) + } + if (is_windowed) { st <- prepare_trip_stops(gtfs, trip_id, with_departure = TRUE) } else { diff --git a/R/get_trip_segment_duration.R b/R/get_trip_segment_duration.R index 453e8fcc..1d412450 100644 --- a/R/get_trip_segment_duration.R +++ b/R/get_trip_segment_duration.R @@ -32,6 +32,10 @@ #' [convert_time_to_seconds()]) are used as-is, not recalculated from the time #' strings. #' +#' Times after midnight written as early morning times (e.g. `"00:30:00"` +#' instead of `"24:30:00"`) are fixed with [fix_overnight_times()] before the +#' durations are calculated. +#' #' `from` and `to` are compared with the departure time of the first stop of #' each segment, read as the durations are (from `departure_time_secs`, if #' present), so use times past `"24:00:00"` for departures after midnight. @@ -99,6 +103,11 @@ get_trip_segment_duration <- function(gtfs, c("character", "character", "character", "integer") ) + # times after midnight written as early morning times are fixed first, so + # they don't go back + + gtfs <- fix_overnight_in_call(gtfs, trip_id) + # select the trips to get the segments' durations of and raise a warning if # a given trip_id doesn't exist in 'stop_times' diff --git a/R/get_trip_speed.R b/R/get_trip_speed.R index 492a5764..b43fcb62 100644 --- a/R/get_trip_speed.R +++ b/R/get_trip_speed.R @@ -45,7 +45,11 @@ #' strings. #' #' `from` and `to` select the trips (or segments) as in [get_trip_length()], -#' from the `departure_time` strings, even if `_secs` columns exist. +#' from the `departure_time` strings, even if `_secs` columns exist. Times +#' after midnight written as early morning times (e.g. `"00:30:00"` instead of +#' `"24:30:00"`) are first fixed with [fix_overnight_times()], if +#' `stop_sequence` is an integer column, so lengths, durations and the time of +#' day all use the fixed times. #' #' @seealso [get_route_speed()], [set_trip_speed()], [set_route_speed()], #' [get_trip_length()], [get_trip_duration()], [get_trip_segment_duration()] @@ -138,6 +142,14 @@ get_trip_speed <- function( method <- map_deprecated_file(file, "get_trip_speed") } + # times after midnight written as early morning times are fixed first, so + # get_trip_length() selects the segments within the time of day from the + # same times as the durations, and the duration functions don't fix them + # again. without an integer stop_sequence the times can't be ordered, so + # they are used as written + + gtfs <- fix_overnight_in_call(gtfs, trip_id) + # get_trip_length() keeps only the trips or segments within the time of # day, so the durations joined to them below are filtered too diff --git a/R/get_vkt.R b/R/get_vkt.R index 78717da9..865f550a 100644 --- a/R/get_vkt.R +++ b/R/get_vkt.R @@ -32,7 +32,9 @@ #' its service runs, according to the `calendar` and `calendar_dates` tables. #' Departures are counted as in [get_route_frequency()]: trips listed in the #' `frequencies` table depart once per headway, the other trips depart once, -#' and departures after midnight belong to the previous service day. Unlike in +#' and departures after midnight belong to the previous service day (times +#' after midnight written as early morning times are fixed first, as in +#' [get_route_frequency()]). Unlike in #' [get_route_frequency()], the time of day includes `to`, as in #' [get_trip_length()]. An error is raised if the `frequencies` table has #' invalid entries for trips that run on the counted days. diff --git a/R/get_vmt.R b/R/get_vmt.R index 4db542b0..ed64e99d 100644 --- a/R/get_vmt.R +++ b/R/get_vmt.R @@ -17,7 +17,8 @@ #' @section Details: #' The VMT is the vehicle-kilometres travelled calculated by [get_vkt()], #' converted to international miles (1 mile = 1.609344 km). See [get_vkt()] -#' for how trips, departures, days and trip lengths are counted. +#' for how trips, departures, days and trip lengths are counted, and how +#' times after midnight written as early morning times are fixed. #' #' @seealso [get_vkt()], [get_route_frequency()], [get_trip_length()] #' diff --git a/R/interpolate_stop_times.R b/R/interpolate_stop_times.R index c5d737a1..0273d3ea 100644 --- a/R/interpolate_stop_times.R +++ b/R/interpolate_stop_times.R @@ -50,8 +50,10 @@ #' - stops between timepoints whose distance can't be calculated, e.g. trips #' not linked to a shape or stops without coordinates; #' - stops between timepoints in which the arrival is earlier than the -#' previous departure, usually times after midnight written as -#' `"00:30:00"` instead of `"24:30:00"`. +#' previous departure. Times after midnight written as early morning times +#' (e.g. `"00:30:00"` instead of `"24:30:00"`) are first fixed with +#' [fix_overnight_times()] in every trip, so the returned GTFS has them +#' fixed, and only times that go back by 12 hours or less remain. #' #' Times are blank when they are `NA`, empty or whitespace-only strings, or #' `"NA"`. Malformed time strings are left unchanged (unless overwritten with @@ -81,7 +83,7 @@ #' head() #' #' # distances measured along the trips' shapes. some trips of this feed have -#' # times past midnight written as "00:xx:xx", so they are left blank +#' # times past midnight written as "00:xx:xx", which are fixed first #' interpolated_gtfs <- suppressWarnings(interpolate_stop_times(gtfs)) #' interpolated_gtfs$stop_times |> #' filter(trip_id == trip) |> @@ -125,6 +127,11 @@ interpolate_stop_times <- function( c("character", "integer", "character", "character") ) + # times after midnight written as early morning times are fixed first, so + # they don't go back. the returned feed keeps the fixed times + + gtfs <- fix_overnight_in_call(gtfs) + stop_times <- gtfs$stop_times if (!is.null(trip_id)) { @@ -241,8 +248,9 @@ interpolate_stop_times <- function( p <- p[is_inside] q <- q[is_inside] - # spans in which the arrival is earlier than the previous departure (e.g. - # times after midnight written as "00:xx:xx") are left blank + # spans in which the arrival is earlier than the previous departure (times + # that go back by 12 hours or less, since the others were fixed) are left + # blank is_backwards <- t_arr[q] < t_dep[p] back_trips <- unique(tid[i[is_backwards & !is_overwritten[i]]]) @@ -442,8 +450,9 @@ warn_times_not_interpolated <- function(n_left, left_trips, back_trips) { message, "i" = paste0( "{length(back_trips)} trip{?s} {?has/have} an arrival earlier than ", - "the previous departure, e.g. times after midnight written as ", - "{.val 00:30:00} instead of {.val 24:30:00}: {.val {back_trips}}." + "the previous departure (times that go back by 12 hours or less are ", + "not taken as times after midnight, see {.fn fix_overnight_times}): ", + "{.val {back_trips}}." ) ) } diff --git a/R/set_dwell_time.R b/R/set_dwell_time.R index f4a2d0f4..fa56992a 100644 --- a/R/set_dwell_time.R +++ b/R/set_dwell_time.R @@ -60,14 +60,19 @@ #' stop only changes how long before each departure the vehicle arrives there, #' while changes at later stops lengthen or shorten every departure. #' -#' `from` and `to` are compared with the `arrival_time` of each visit as -#' written in `stop_times` before any change (template times, for trips listed -#' in `frequencies`), so use times past `"24:00:00"` for arrivals after -#' midnight. A shift caused by an earlier visit may thus move a changed visit -#' out of the time of day, or another visit into it. Visits with a blank +#' Times after midnight written as early morning times (e.g. `"00:30:00"` +#' instead of `"24:30:00"`) are first fixed with [fix_overnight_times()] in +#' every trip, so the returned GTFS has them fixed (with `by_reference = TRUE`, +#' even if an error is then raised). +#' +#' `from` and `to` are compared with the `arrival_time` of each visit as written +#' in `stop_times` before any change other than this fix (template times, for +#' trips listed in `frequencies`), so use times past `"24:00:00"` for arrivals +#' after midnight. A shift caused by an earlier visit may thus move a changed +#' visit out of the time of day, or another visit into it. Visits with a blank #' arrival time are never selected when `from` or `to` is given. When the time #' of day contains no visit of the specified trips to the specified stops, the -#' GTFS is returned unchanged. +#' GTFS is returned with no change other than this fix. #' #' `stop_id`s are matched against the `stop_id`s in `stop_times`, which are #' never stations: to set the dwell times at a station, pass its children that @@ -222,6 +227,13 @@ change_dwell_times <- function( stop_times <- data.table::copy(gtfs$stop_times) } + # times after midnight written as early morning times are fixed first, in + # the table that is edited, so they don't go back and the returned feed + # keeps them fixed + + gtfs$stop_times <- stop_times + gtfs <- fix_overnight_in_call(gtfs, by_reference = TRUE) + selected_rows <- select_dwell_time_calls(stop_times, trip_id, stop_id) # ids missing from 'stop_times' were already reported by the helper diff --git a/R/set_max_speed.R b/R/set_max_speed.R index 3a7e2c9d..d3fafd84 100644 --- a/R/set_max_speed.R +++ b/R/set_max_speed.R @@ -44,9 +44,11 @@ #' Each span faster than `max_speed`, or with a zero duration, has its duration #' raised to its length divided by `max_speed`, rounded up to the next second. #' All the later times of the trip are shifted by the accumulated extra time, -#' so dwell times are kept. Spans whose arrival is earlier than the previous -#' departure (usually times after midnight written as `"00:30:00"` instead of -#' `"24:30:00"`) are left unchanged, with a warning. An error is raised if a +#' so dwell times are kept. Times after midnight written as early morning +#' times (e.g. `"00:30:00"` instead of `"24:30:00"`) are first fixed with +#' [fix_overnight_times()] in every trip, so the returned GTFS has them fixed. +#' Spans whose arrival is still earlier than the previous departure (by 12 +#' hours or less) are left unchanged, with a warning. An error is raised if a #' shift would result in a time later than `"9999:59:59"`. Only `stop_times` #' is modified; its existing `_secs` columns are updated to match. #' @@ -59,8 +61,8 @@ #' warning if they were given in `trip_id` or `route_id`. #' #' `from` and `to` are compared with the departure at the segment's first stop -#' (or its arrival, if the departure is blank) as written in `stop_times` -#' (template times, for trips listed in `frequencies`), so use times past +#' (or its arrival, if the departure is blank) as written in `stop_times`, once +#' fixed (template times, for trips listed in `frequencies`), so use times past #' `"24:00:00"` for departures after midnight. Trips without a time there are #' left unchanged. #' @@ -154,6 +156,11 @@ set_max_speed <- function( gtfsio::assert_field_class(gtfs, "stop_times", "stop_id", "character") } + # times after midnight written as early morning times are fixed first, so + # they don't go back. the returned feed keeps the fixed times + + gtfs <- fix_overnight_in_call(gtfs) + # calculations use km/h max_kmh <- max_speed * speed_unit_factors[[unit]] @@ -384,8 +391,8 @@ set_max_speed <- function( "{.val {abbreviated$shown}}{abbreviated$more}." ), "i" = paste0( - "This usually happens when times after midnight are written as ", - "{.val 00:30:00} instead of {.val 24:30:00}." + "Times that go back by 12 hours or less are not taken as times ", + "after midnight (see {.fn fix_overnight_times})." ) ), class = "gtfstools_backwards_times" diff --git a/R/set_min_dwell_time.R b/R/set_min_dwell_time.R index 5a5001cd..329da1f5 100644 --- a/R/set_min_dwell_time.R +++ b/R/set_min_dwell_time.R @@ -34,8 +34,8 @@ #' #' Negative dwell times (departure earlier than arrival) are raised like any #' other shorter dwell time, so a `min_dwell_time` of 0 raises only these. -#' When they come from times after midnight written as `"00:30:00"` instead of -#' `"24:30:00"`, the later times of the trip are shifted by about a day. +#' Times after midnight written as early morning times are fixed first, as in +#' [set_dwell_time()], so they don't result in negative dwell times. #' #' Otherwise it works as [set_dwell_time()]. #' diff --git a/R/set_route_frequency.R b/R/set_route_frequency.R index 36d9e34e..a71b4416 100644 --- a/R/set_route_frequency.R +++ b/R/set_route_frequency.R @@ -33,6 +33,10 @@ #' headway (exactly, when the length of the time of day is a multiple of #' `headway`). Departures after midnight are listed with times past #' "24:00:00", so use a time of day such as "24:00:00"-"26:00:00" for them. +#' Times after midnight written as early morning times (e.g. `"00:30:00"` +#' instead of `"24:30:00"`), and `end_time`s earlier than `start_time`s in +#' `frequencies`, are first fixed with [fix_overnight_times()], so the +#' returned GTFS has them fixed. #' #' The function changes existing service, it does not create it: groups with #' no departure within the time of day are not modified, and scheduled trips @@ -143,6 +147,12 @@ set_route_frequency <- function(gtfs, rep("character", 3) ) + # times after midnight written as early morning times are fixed first, so + # the departures and the clipped 'frequencies' entries below use the same + # times. the returned feed keeps them fixed + + gtfs <- fix_overnight_in_call(gtfs, frequencies = TRUE) + # select the trips of the relevant routes and raise warning if a given # route_id doesn't exist in 'trips'. the departures are grouped by route, # service and direction (if available), as in get_route_frequency() diff --git a/R/set_trip_speed.R b/R/set_trip_speed.R index 1654867b..f789ce10 100644 --- a/R/set_trip_speed.R +++ b/R/set_trip_speed.R @@ -60,8 +60,13 @@ #' `first_stop`, `last_stop`, `from` and `to`, a blank departure at the trip's #' first stop leaves all the times of the trip blank, without a warning. #' +#' Times after midnight written as early morning times (e.g. `"00:30:00"` +#' instead of `"24:30:00"`) are first fixed with [fix_overnight_times()] in +#' every trip, so the returned GTFS has them fixed (with `by_reference = TRUE`, +#' even if an error is then raised). +#' #' `from` and `to` are compared with the `departure_time` at `first_stop` as -#' written in `stop_times` (template times, for trips listed in +#' written in `stop_times`, once fixed (template times, for trips listed in #' `frequencies`), so use times past `"24:00:00"` for departures after #' midnight; trips whose departure there is blank are then left unchanged. #' Existing `_secs` columns in `stop_times` are updated to match. @@ -259,6 +264,13 @@ set_trip_speed <- function( stop_times <- data.table::copy(gtfs$stop_times) } + # times after midnight written as early morning times are fixed first, in + # the table that is edited, so they don't go back and the returned feed + # keeps them fixed + + gtfs$stop_times <- stop_times + gtfs <- fix_overnight_in_call(gtfs, by_reference = TRUE) + # select the rows of the given trips, ordered as in get_trip_length(), so # that the segment starting at the n-th stop of a trip is its n-th segment. # 'which = TRUE' returns row numbers of stop_times (.I in a j filtered by i diff --git a/_pkgdown.yml b/_pkgdown.yml index 7258df69..46cb7288 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -41,6 +41,7 @@ reference: - remove_duplicates - remove_unused_ids - fix_gtfs + - fix_overnight_times - set_dwell_time - set_min_dwell_time - set_max_speed diff --git a/man/build_graph.Rd b/man/build_graph.Rd index 8bdd1b97..2e968025 100644 --- a/man/build_graph.Rd +++ b/man/build_graph.Rd @@ -90,7 +90,10 @@ are not included. The trips of every \code{service_id} are pooled, so the feed should usually be filtered to a single day first, with \code{\link[=filter_by_date]{filter_by_date()}} or \code{\link[=filter_by_weekday]{filter_by_weekday()}}. Departures after midnight have times past "24:00:00", so use a time of day such as "24:00:00"-"26:00:00" for them. -Trips whose times wrongly go back to "00:00:00" after midnight are placed +Times after midnight written as early morning times (e.g. \code{"00:30:00"} +instead of \code{"24:30:00"}) are first fixed with \code{\link[=fix_overnight_times]{fix_overnight_times()}}, as +are \code{end_time}s earlier than \code{start_time}s in \code{frequencies}, so these trips +are placed at their real times. Trips whose times still go back are placed by their earliest time. Within a trip, consecutive \code{stop_times} entries at the same stop are one diff --git a/man/filter_by_time_of_day.Rd b/man/filter_by_time_of_day.Rd index 4a670216..0840554b 100644 --- a/man/filter_by_time_of_day.Rd +++ b/man/filter_by_time_of_day.Rd @@ -48,6 +48,12 @@ of the other tables related to the trips that remain. The time of day includes both \code{from} and \code{to}. +Times after midnight written as early morning times (e.g. \code{"00:30:00"} +instead of \code{"24:30:00"}), and \code{end_time}s earlier than \code{start_time}s in +\code{frequencies}, are first fixed with \code{\link[=fix_overnight_times]{fix_overnight_times()}}, so these +times are compared with the time of day as they should be, and the +returned GTFS has them fixed. + Trips listed in \code{frequencies} are filtered through their \code{frequencies} entries; their \code{stop_times} entries are templates and are kept whatever their times. With \code{keep = TRUE}, the entries that overlap the time of day diff --git a/man/fix_gtfs.Rd b/man/fix_gtfs.Rd index 3ce0c9eb..829c2fe3 100644 --- a/man/fix_gtfs.Rd +++ b/man/fix_gtfs.Rd @@ -35,10 +35,10 @@ The fixed GTFS object. } \description{ Cleans a GTFS feed in one call, by removing duplicated entries and unused -ids, optionally capping the speed of trips, filling blank stop times, -optionally setting a minimum dwell time at intermediate stops and -recalculating the distances travelled along shapes (replacing any existing -ones). +ids, fixing times after midnight written as early morning times, optionally +capping the speed of trips, filling blank stop times, optionally setting a +minimum dwell time at intermediate stops and recalculating the distances +travelled along shapes (replacing any existing ones). } \section{Details}{ @@ -49,11 +49,15 @@ The feed goes through the following steps, in this order: entries of every table that no remaining trip uses (see its documentation for the exceptions). Note that this removes data from the feed; +\item \code{\link[=fix_overnight_times]{fix_overnight_times()}} rewrites the times after midnight written as +early morning times (e.g. \code{"00:30:00"} instead of \code{"24:30:00"}), so the +next steps work on these trips too. Only times that go back by more +than 12 hours are taken as times after midnight, and the times that +still go back are left unchanged, with a warning; \item \code{\link[=set_max_speed]{set_max_speed()}}, only if \code{max_speed} is given, slows down the parts of trips that are faster than \code{max_speed}. Only spans between stops with -known times are capped, and spans that end before they start (e.g. -times after midnight written as \code{"00:30:00"}) are left unchanged, with -a warning; +known times are capped, and spans that still end before they start are +left unchanged, with a warning; \item \code{\link[=interpolate_stop_times]{interpolate_stop_times()}} fills blank stop times from the times of the stops around them. Speeds are capped before interpolating, so the filled times follow the capped times. Interpolated times are rounded to the @@ -68,9 +72,7 @@ in \code{frequencies}, every departure gets longer). This is done after filling blank times, so the filled stops also get the minimum. Only the stops between the first and the last stop with times of each trip get the minimum, so trips keep their start times. Stops with negative dwell -times (departures earlier than arrivals) also get it, see -\code{\link[=set_min_dwell_time]{set_min_dwell_time()}} for times after midnight written as -\code{"00:30:00"}; +times (departures earlier than arrivals) also get it; \item \code{\link[=interpolate_shape_dist_traveled]{interpolate_shape_dist_traveled()}}, only if the feed has a \code{shapes} table and \code{trips} has a \code{shape_id} column, replaces the \code{shape_dist_traveled} columns of \code{stop_times} and \code{shapes} with the @@ -115,7 +117,8 @@ fixed_gtfs$stop_times |> \dontshow{\}) # examplesIf} } \seealso{ -\code{\link[=remove_duplicates]{remove_duplicates()}}, \code{\link[=remove_unused_ids]{remove_unused_ids()}}, \code{\link[=set_max_speed]{set_max_speed()}}, +\code{\link[=remove_duplicates]{remove_duplicates()}}, \code{\link[=remove_unused_ids]{remove_unused_ids()}}, +\code{\link[=fix_overnight_times]{fix_overnight_times()}}, \code{\link[=set_max_speed]{set_max_speed()}}, \code{\link[=interpolate_stop_times]{interpolate_stop_times()}}, \code{\link[=set_min_dwell_time]{set_min_dwell_time()}}, \code{\link[=interpolate_shape_dist_traveled]{interpolate_shape_dist_traveled()}} } diff --git a/man/fix_overnight_frequencies.Rd b/man/fix_overnight_frequencies.Rd new file mode 100644 index 00000000..e136c4ba --- /dev/null +++ b/man/fix_overnight_frequencies.Rd @@ -0,0 +1,19 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/fix_overnight_times.R +\name{fix_overnight_frequencies} +\alias{fix_overnight_frequencies} +\title{Fix the end times after midnight of \code{frequencies}} +\usage{ +fix_overnight_frequencies(gtfs) +} +\arguments{ +\item{gtfs}{A GTFS object, whose \code{frequencies} is copied before being +changed.} +} +\value{ +The GTFS object. +} +\description{ +Fix the end times after midnight of \code{frequencies} +} +\keyword{internal} diff --git a/man/fix_overnight_in_call.Rd b/man/fix_overnight_in_call.Rd new file mode 100644 index 00000000..afaf6a4b --- /dev/null +++ b/man/fix_overnight_in_call.Rd @@ -0,0 +1,34 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/fix_overnight_times.R +\name{fix_overnight_in_call} +\alias{fix_overnight_in_call} +\title{Fix the times after midnight silently} +\usage{ +fix_overnight_in_call( + gtfs, + trip_id = NULL, + frequencies = FALSE, + by_reference = FALSE +) +} +\arguments{ +\item{gtfs}{A GTFS object.} + +\item{trip_id}{A character vector with the \code{trip_id}s to fix, or \code{NULL} +for every trip.} + +\item{frequencies}{Whether to fix the \code{end_time}s of \code{frequencies} too.} + +\item{by_reference}{Whether to write the fixed times into +\code{gtfs$stop_times} itself.} +} +\value{ +The GTFS object. +} +\description{ +Fixes the times after midnight of \code{stop_times} (and of \code{frequencies}) +silently, as the functions that use the times do before using them. A +feed whose times were already fixed is returned unchanged, without being +copied, so a function called by another one doesn't fix them again. +} +\keyword{internal} diff --git a/man/fix_overnight_stop_times.Rd b/man/fix_overnight_stop_times.Rd new file mode 100644 index 00000000..a85cf410 --- /dev/null +++ b/man/fix_overnight_stop_times.Rd @@ -0,0 +1,42 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/fix_overnight_times.R +\name{fix_overnight_stop_times} +\alias{fix_overnight_stop_times} +\title{Fix the times after midnight of \code{stop_times}} +\usage{ +fix_overnight_stop_times( + gtfs, + trip_id = NULL, + warn = TRUE, + by_reference = FALSE, + detect_only = FALSE +) +} +\arguments{ +\item{gtfs}{A GTFS object whose \code{stop_times} has the \code{trip_id}, +\code{stop_sequence}, \code{arrival_time} and \code{departure_time} fields, of the +classes asserted by \code{\link[=fix_overnight_times]{fix_overnight_times()}}.} + +\item{trip_id}{A character vector with the \code{trip_id}s to fix, or \code{NULL} +(the default) for every trip. Each trip is fixed on its own, so the +functions that only use some trips fix only these.} + +\item{warn}{Whether to warn about the times that still go back after the +fix, and about malformed times.} + +\item{by_reference}{Whether to write the fixed times into +\code{gtfs$stop_times} itself. If \code{FALSE}, the table is copied before being +changed.} + +\item{detect_only}{Whether to only detect the times to fix, without fixing +them.} +} +\value{ +The GTFS object, or, with \code{detect_only = TRUE}, \code{TRUE} if any time +would be fixed and \code{FALSE} otherwise. +} +\description{ +Also used by the functions that calculate speeds and durations, which fix +the times silently before using them. +} +\keyword{internal} diff --git a/man/fix_overnight_times.Rd b/man/fix_overnight_times.Rd new file mode 100644 index 00000000..1df27ede --- /dev/null +++ b/man/fix_overnight_times.Rd @@ -0,0 +1,88 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/fix_overnight_times.R +\name{fix_overnight_times} +\alias{fix_overnight_times} +\title{Fix times after midnight written as early morning times} +\usage{ +fix_overnight_times(gtfs) +} +\arguments{ +\item{gtfs}{A GTFS object, as created by \code{\link[=read_gtfs]{read_gtfs()}}.} +} +\value{ +A GTFS object with the time columns of its \code{stop_times} and the +\code{end_time} column of its \code{frequencies} fixed. +} +\description{ +Rewrites the times after midnight of trips that run past it, when they are +written as early morning times (e.g. \code{"00:10:00"} after \code{"23:50:00"}), as +times past \code{"24:00:00"} (e.g. \code{"24:10:00"}), as required by the GTFS +specification. All gtfstools functions that calculate or edit durations +and speeds or use information on time of day use \code{fix_overnight_times()} +internally to fix the overnight times. +} +\section{Details}{ + +The arrival and departure times of each trip are read in order of +\code{stop_sequence}, ignoring blank times. A time more than 12 hours earlier +than the previous time of its trip is taken as a time after midnight, so +it and the later times of the trip are moved one day later (e.g. +\code{"00:10:00"} becomes \code{"24:10:00"}). Trips that run past a second midnight +are moved two days, and so on. A time more than 12 hours later than the +previous time of its trip undoes one of these days, so feeds that mix both +forms are also fixed (e.g. \code{"24:30:00"}, \code{"00:40:00"}, \code{"24:50:00"} +becomes \code{"24:30:00"}, \code{"24:40:00"}, \code{"24:50:00"}), and trips whose times +already follow the specification are left unchanged. Blank times are kept +blank. Only \code{stop_times} and \code{frequencies} are modified; their existing +\verb{_secs} columns are updated to match. + +Times earlier than the previous time of their trip by 12 hours or less are +left unchanged, with a warning. Times are never moved earlier, so trips +with known times more than 12 hours apart that follow the specification +(e.g. \code{"06:00:00"}, \code{"19:30:00"}) are left unchanged. Rows with a missing +\code{trip_id} or \code{stop_sequence} are left unchanged, and stops with duplicated +\code{stop_sequence}s are read in the order of their rows (see +\code{\link[=remove_duplicates]{remove_duplicates()}}). + +Trips whose times are all written after midnight as early morning times +(e.g. a trip of the previous service day departing at \code{"00:30:00"}) can't +be told apart from trips of the service day and are left unchanged. + +In \code{frequencies}, an \code{end_time} earlier than the \code{start_time} of its row +is moved by whole days to the first time later than the \code{start_time} +(e.g. \code{"23:00:00"}-\code{"01:00:00"} becomes \code{"23:00:00"}-\code{"25:00:00"}). A +\code{start_time} written as an early morning time is left unchanged. The +template times of trips listed in \code{frequencies} are fixed as any other +times, which doesn't change their departures, since only the differences +between them are used. + +An error is raised if a fixed time would be later than \code{"9999:59:59"}. +} + +\examples{ +\dontshow{if (requireNamespace("dplyr", quietly = TRUE)) withAutoprint(\{ # examplesIf} +\dontshow{ + old_dt_threads <- data.table::setDTthreads(1) +} +library(dplyr) + +data_path <- system.file("extdata/poa_gtfs.zip", package = "gtfstools") + +gtfs <- read_gtfs(data_path) + +# this trip runs past midnight, but its last time is written as "00:02:00" +gtfs$stop_times |> + filter(trip_id == "T2-1@1#2310", arrival_time != "") |> + select(trip_id, stop_sequence, arrival_time, departure_time) + +fixed_gtfs <- fix_overnight_times(gtfs) + +fixed_gtfs$stop_times |> + filter(trip_id == "T2-1@1#2310", arrival_time != "") |> + select(trip_id, stop_sequence, arrival_time, departure_time) +\dontshow{data.table::setDTthreads(old_dt_threads)} +\dontshow{\}) # examplesIf} +} +\seealso{ +\code{\link[=fix_gtfs]{fix_gtfs()}}, \code{\link[=convert_time_to_seconds]{convert_time_to_seconds()}} +} diff --git a/man/get_dwell_time.Rd b/man/get_dwell_time.Rd index f3da7270..d797e20b 100644 --- a/man/get_dwell_time.Rd +++ b/man/get_dwell_time.Rd @@ -62,8 +62,12 @@ Blank times (\code{NA} dwell times) can be filled beforehand with \code{\link[=interpolate_stop_times]{interpolate_stop_times()}}. Negative dwell times (departure earlier than arrival) are returned as they are. +Times after midnight written as early morning times (e.g. \code{"00:30:00"} +instead of \code{"24:30:00"}) are fixed with \code{\link[=fix_overnight_times]{fix_overnight_times()}} before the +dwell times are calculated. + \code{from} and \code{to} are compared with the \code{arrival_time} of each visit as -written in \code{stop_times} (template times, for trips listed in +written in \code{stop_times}, once fixed (template times, for trips listed in \code{frequencies}), so use times past \code{"24:00:00"} for arrivals after midnight. Visits with a blank arrival time are never included when \code{from} or \code{to} is given. diff --git a/man/get_route_frequency.Rd b/man/get_route_frequency.Rd index 99814cdb..d7db968c 100644 --- a/man/get_route_frequency.Rd +++ b/man/get_route_frequency.Rd @@ -50,6 +50,12 @@ A departure is counted when it happens at or after \code{from} and before \code{ (unlike in \code{\link[=filter_by_time_of_day]{filter_by_time_of_day()}}, which also keeps entries at \code{to}). Departures after midnight are listed with times past "24:00:00", so use a time of day such as "24:00:00"-"26:00:00" to count them. +Times after midnight written as early morning times (e.g. \code{"00:30:00"} +instead of \code{"24:30:00"}), and \code{end_time}s earlier than \code{start_time}s in +\code{frequencies}, are first fixed with \code{\link[=fix_overnight_times]{fix_overnight_times()}}, so these +trips are counted at their real times. Without an integer \code{stop_sequence} +column, the times of \code{stop_times} can't be ordered and are used as +written. The mean headway is the length of the time of day divided by the number of departures, so it overstates the actual headway when service starts or ends diff --git a/man/get_stop_frequency.Rd b/man/get_stop_frequency.Rd index c0622a72..f90a10a7 100644 --- a/man/get_stop_frequency.Rd +++ b/man/get_stop_frequency.Rd @@ -70,6 +70,10 @@ that this differs from \code{\link[=filter_by_time_of_day]{filter_by_time_of_day times are equal to \code{to}. Departures after midnight are listed with times greater than "24:00:00" and belong to the previous service day, so a time of day such as "24:00:00"-"26:00:00" should be used to account for them. +Times after midnight written as early morning times (e.g. \code{"00:30:00"} +instead of \code{"24:30:00"}), and \code{end_time}s earlier than \code{start_time}s in +\code{frequencies}, are first fixed with \code{\link[=fix_overnight_times]{fix_overnight_times()}}, so these +departures are counted at their real times. The mean headway is the length of the time of day divided by the number of departures. It is an average over the whole time of day, so it is larger diff --git a/man/get_summary.Rd b/man/get_summary.Rd index 965597e5..d39ac12b 100644 --- a/man/get_summary.Rd +++ b/man/get_summary.Rd @@ -122,6 +122,11 @@ issues in the GTFS feed. To do that, use \code{\link[=validate_gtfs]{validate_gt The report requires the \code{{ggplot2}}, \code{{knitr}} and \code{{rmarkdown}} packages and \href{https://pandoc.org/}{Pandoc}. +Times after midnight written as early morning times (e.g. \code{"00:30:00"} +instead of \code{"24:30:00"}), and \code{end_time}s earlier than \code{start_time}s in +\code{frequencies}, are first fixed with \code{\link[=fix_overnight_times]{fix_overnight_times()}}, if +\code{stop_sequence} is an integer column. + The time-of-day filter keeps whole trips (as \code{\link[=filter_by_time_of_day]{filter_by_time_of_day()}} with \code{full_trips = TRUE}), so trip speeds and lengths refer to whole trips. diff --git a/man/get_trip_duration.Rd b/man/get_trip_duration.Rd index c56cbf3d..d9af3e57 100644 --- a/man/get_trip_duration.Rd +++ b/man/get_trip_duration.Rd @@ -42,6 +42,11 @@ Existing \verb{_secs} columns in \code{stop_times} (e.g. created with \code{\link[=convert_time_to_seconds]{convert_time_to_seconds()}}) are used as-is, not recalculated from the time strings. +Times after midnight written as early morning times (e.g. \code{"00:30:00"} +instead of \code{"24:30:00"}) are fixed with \code{\link[=fix_overnight_times]{fix_overnight_times()}} before the +durations are calculated, if \code{stop_sequence} is an integer column (without +it, the times can't be ordered and are used as written). + \code{from} and \code{to} are compared with the departure time of the first stop of each trip, in \code{stop_sequence} order (which must then be an integer column). The time is read as the durations are: from \code{departure_time_secs}, if diff --git a/man/get_trip_length.Rd b/man/get_trip_length.Rd index 5696df85..1fa00935 100644 --- a/man/get_trip_length.Rd +++ b/man/get_trip_length.Rd @@ -98,6 +98,9 @@ Lengths are great-circle (haversine) distances on the sphere used by \code{from} and \code{to} are compared with the \code{departure_time} of the first stop of each trip (in \code{stop_sequence} order) or segment, as written in \code{stop_times}, so use times past \code{"24:00:00"} for departures after midnight. +With \code{by = "segment"}, times after midnight written as early morning times +(e.g. \code{"00:30:00"} instead of \code{"24:30:00"}) are first fixed with +\code{\link[=fix_overnight_times]{fix_overnight_times()}}; a trip's first departure is never changed by it. For trips listed in \code{frequencies}, these are template times, not the times of each departure. Trips and segments whose departure is blank are never included when \code{from} or \code{to} is given. diff --git a/man/get_trip_segment_duration.Rd b/man/get_trip_segment_duration.Rd index 347ce985..715cbd37 100644 --- a/man/get_trip_segment_duration.Rd +++ b/man/get_trip_segment_duration.Rd @@ -54,6 +54,10 @@ Existing \verb{_secs} columns in \code{stop_times} (e.g. created with \code{\link[=convert_time_to_seconds]{convert_time_to_seconds()}}) are used as-is, not recalculated from the time strings. +Times after midnight written as early morning times (e.g. \code{"00:30:00"} +instead of \code{"24:30:00"}) are fixed with \code{\link[=fix_overnight_times]{fix_overnight_times()}} before the +durations are calculated. + \code{from} and \code{to} are compared with the departure time of the first stop of each segment, read as the durations are (from \code{departure_time_secs}, if present), so use times past \code{"24:00:00"} for departures after midnight. diff --git a/man/get_trip_speed.Rd b/man/get_trip_speed.Rd index eaf1c25a..27590271 100644 --- a/man/get_trip_speed.Rd +++ b/man/get_trip_speed.Rd @@ -85,7 +85,11 @@ Existing \verb{_secs} columns in \code{stop_times} (e.g. created with strings. \code{from} and \code{to} select the trips (or segments) as in \code{\link[=get_trip_length]{get_trip_length()}}, -from the \code{departure_time} strings, even if \verb{_secs} columns exist. +from the \code{departure_time} strings, even if \verb{_secs} columns exist. Times +after midnight written as early morning times (e.g. \code{"00:30:00"} instead of +\code{"24:30:00"}) are first fixed with \code{\link[=fix_overnight_times]{fix_overnight_times()}}, if +\code{stop_sequence} is an integer column, so lengths, durations and the time of +day all use the fixed times. } \examples{ diff --git a/man/get_vkt.Rd b/man/get_vkt.Rd index 0e4952eb..b011d548 100644 --- a/man/get_vkt.Rd +++ b/man/get_vkt.Rd @@ -45,7 +45,9 @@ times its number of departures times the number of counted days on which its service runs, according to the \code{calendar} and \code{calendar_dates} tables. Departures are counted as in \code{\link[=get_route_frequency]{get_route_frequency()}}: trips listed in the \code{frequencies} table depart once per headway, the other trips depart once, -and departures after midnight belong to the previous service day. Unlike in +and departures after midnight belong to the previous service day (times +after midnight written as early morning times are fixed first, as in +\code{\link[=get_route_frequency]{get_route_frequency()}}). Unlike in \code{\link[=get_route_frequency]{get_route_frequency()}}, the time of day includes \code{to}, as in \code{\link[=get_trip_length]{get_trip_length()}}. An error is raised if the \code{frequencies} table has invalid entries for trips that run on the counted days. diff --git a/man/get_vmt.Rd b/man/get_vmt.Rd index c0ca5b6b..f3e5af95 100644 --- a/man/get_vmt.Rd +++ b/man/get_vmt.Rd @@ -41,7 +41,8 @@ dates and to a time of day. The VMT is the vehicle-kilometres travelled calculated by \code{\link[=get_vkt]{get_vkt()}}, converted to international miles (1 mile = 1.609344 km). See \code{\link[=get_vkt]{get_vkt()}} -for how trips, departures, days and trip lengths are counted. +for how trips, departures, days and trip lengths are counted, and how +times after midnight written as early morning times are fixed. } \examples{ diff --git a/man/has_overnight_trips.Rd b/man/has_overnight_trips.Rd new file mode 100644 index 00000000..8d1cc280 --- /dev/null +++ b/man/has_overnight_trips.Rd @@ -0,0 +1,25 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/fix_overnight_times.R +\name{has_overnight_trips} +\alias{has_overnight_trips} +\title{Check whether a feed has trips with times after midnight to fix} +\usage{ +has_overnight_trips(gtfs) +} +\arguments{ +\item{gtfs}{A GTFS object.} +} +\value{ +\code{TRUE} if \code{\link[=fix_overnight_times]{fix_overnight_times()}} would change any time, \code{FALSE} +otherwise. +} +\description{ +Detects the trips whose times after midnight are written as early morning +times (e.g. \code{"00:30:00"} instead of \code{"24:30:00"}), as \code{\link[=fix_overnight_times]{fix_overnight_times()}} +does, without fixing them: trips with a time more than 12 hours earlier than +the previous time of the trip, and \code{frequencies} entries whose \code{end_time} is +earlier than their \code{start_time}. Trips already written with times past +\code{"24:00:00"} don't count. Tables whose fields can't be checked (e.g. a +\code{stop_times} without an integer \code{stop_sequence}) are skipped. +} +\keyword{internal} diff --git a/man/interpolate_stop_times.Rd b/man/interpolate_stop_times.Rd index 63a7f094..356c1a80 100644 --- a/man/interpolate_stop_times.Rd +++ b/man/interpolate_stop_times.Rd @@ -75,8 +75,10 @@ stop of trips with fewer than two timepoints; \item stops between timepoints whose distance can't be calculated, e.g. trips not linked to a shape or stops without coordinates; \item stops between timepoints in which the arrival is earlier than the -previous departure, usually times after midnight written as -\code{"00:30:00"} instead of \code{"24:30:00"}. +previous departure. Times after midnight written as early morning times +(e.g. \code{"00:30:00"} instead of \code{"24:30:00"}) are first fixed with +\code{\link[=fix_overnight_times]{fix_overnight_times()}} in every trip, so the returned GTFS has them +fixed, and only times that go back by 12 hours or less remain. } Times are blank when they are \code{NA}, empty or whitespace-only strings, or @@ -107,7 +109,7 @@ gtfs$stop_times |> head() # distances measured along the trips' shapes. some trips of this feed have -# times past midnight written as "00:xx:xx", so they are left blank +# times past midnight written as "00:xx:xx", which are fixed first interpolated_gtfs <- suppressWarnings(interpolate_stop_times(gtfs)) interpolated_gtfs$stop_times |> filter(trip_id == trip) |> diff --git a/man/set_dwell_time.Rd b/man/set_dwell_time.Rd index 24ea5d30..cf619316 100644 --- a/man/set_dwell_time.Rd +++ b/man/set_dwell_time.Rd @@ -92,14 +92,19 @@ For trips listed in \code{frequencies}, the times in \code{stop_times} are a tem stop only changes how long before each departure the vehicle arrives there, while changes at later stops lengthen or shorten every departure. -\code{from} and \code{to} are compared with the \code{arrival_time} of each visit as -written in \code{stop_times} before any change (template times, for trips listed -in \code{frequencies}), so use times past \code{"24:00:00"} for arrivals after -midnight. A shift caused by an earlier visit may thus move a changed visit -out of the time of day, or another visit into it. Visits with a blank +Times after midnight written as early morning times (e.g. \code{"00:30:00"} +instead of \code{"24:30:00"}) are first fixed with \code{\link[=fix_overnight_times]{fix_overnight_times()}} in +every trip, so the returned GTFS has them fixed (with \code{by_reference = TRUE}, +even if an error is then raised). + +\code{from} and \code{to} are compared with the \code{arrival_time} of each visit as written +in \code{stop_times} before any change other than this fix (template times, for +trips listed in \code{frequencies}), so use times past \code{"24:00:00"} for arrivals +after midnight. A shift caused by an earlier visit may thus move a changed +visit out of the time of day, or another visit into it. Visits with a blank arrival time are never selected when \code{from} or \code{to} is given. When the time of day contains no visit of the specified trips to the specified stops, the -GTFS is returned unchanged. +GTFS is returned with no change other than this fix. \code{stop_id}s are matched against the \code{stop_id}s in \code{stop_times}, which are never stations: to set the dwell times at a station, pass its children that diff --git a/man/set_max_speed.Rd b/man/set_max_speed.Rd index fc178b6b..6bd95122 100644 --- a/man/set_max_speed.Rd +++ b/man/set_max_speed.Rd @@ -78,9 +78,11 @@ in trips not linked to a shape) are left unchanged. Each span faster than \code{max_speed}, or with a zero duration, has its duration raised to its length divided by \code{max_speed}, rounded up to the next second. All the later times of the trip are shifted by the accumulated extra time, -so dwell times are kept. Spans whose arrival is earlier than the previous -departure (usually times after midnight written as \code{"00:30:00"} instead of -\code{"24:30:00"}) are left unchanged, with a warning. An error is raised if a +so dwell times are kept. Times after midnight written as early morning +times (e.g. \code{"00:30:00"} instead of \code{"24:30:00"}) are first fixed with +\code{\link[=fix_overnight_times]{fix_overnight_times()}} in every trip, so the returned GTFS has them fixed. +Spans whose arrival is still earlier than the previous departure (by 12 +hours or less) are left unchanged, with a warning. An error is raised if a shift would result in a time later than \code{"9999:59:59"}. Only \code{stop_times} is modified; its existing \verb{_secs} columns are updated to match. @@ -93,8 +95,8 @@ then \code{last_stop}, or have blank times there, are left unchanged, with a warning if they were given in \code{trip_id} or \code{route_id}. \code{from} and \code{to} are compared with the departure at the segment's first stop -(or its arrival, if the departure is blank) as written in \code{stop_times} -(template times, for trips listed in \code{frequencies}), so use times past +(or its arrival, if the departure is blank) as written in \code{stop_times}, once +fixed (template times, for trips listed in \code{frequencies}), so use times past \code{"24:00:00"} for departures after midnight. Trips without a time there are left unchanged. } diff --git a/man/set_min_dwell_time.Rd b/man/set_min_dwell_time.Rd index 1845f11d..407bfd48 100644 --- a/man/set_min_dwell_time.Rd +++ b/man/set_min_dwell_time.Rd @@ -76,8 +76,8 @@ of the last visit may still be shifted by an earlier visit. Negative dwell times (departure earlier than arrival) are raised like any other shorter dwell time, so a \code{min_dwell_time} of 0 raises only these. -When they come from times after midnight written as \code{"00:30:00"} instead of -\code{"24:30:00"}, the later times of the trip are shifted by about a day. +Times after midnight written as early morning times are fixed first, as in +\code{\link[=set_dwell_time]{set_dwell_time()}}, so they don't result in negative dwell times. Otherwise it works as \code{\link[=set_dwell_time]{set_dwell_time()}}. } diff --git a/man/set_route_frequency.Rd b/man/set_route_frequency.Rd index 7907a0c2..18aa77b1 100644 --- a/man/set_route_frequency.Rd +++ b/man/set_route_frequency.Rd @@ -55,6 +55,10 @@ and before \code{to}) as in \code{\link[=get_route_frequency]{get_route_frequenc headway (exactly, when the length of the time of day is a multiple of \code{headway}). Departures after midnight are listed with times past "24:00:00", so use a time of day such as "24:00:00"-"26:00:00" for them. +Times after midnight written as early morning times (e.g. \code{"00:30:00"} +instead of \code{"24:30:00"}), and \code{end_time}s earlier than \code{start_time}s in +\code{frequencies}, are first fixed with \code{\link[=fix_overnight_times]{fix_overnight_times()}}, so the +returned GTFS has them fixed. The function changes existing service, it does not create it: groups with no departure within the time of day are not modified, and scheduled trips diff --git a/man/set_trip_speed.Rd b/man/set_trip_speed.Rd index cf8ed8b4..bea8fc23 100644 --- a/man/set_trip_speed.Rd +++ b/man/set_trip_speed.Rd @@ -90,8 +90,13 @@ left unchanged, with a warning if they were given in \code{trip_id}. Without \code{first_stop}, \code{last_stop}, \code{from} and \code{to}, a blank departure at the trip's first stop leaves all the times of the trip blank, without a warning. +Times after midnight written as early morning times (e.g. \code{"00:30:00"} +instead of \code{"24:30:00"}) are first fixed with \code{\link[=fix_overnight_times]{fix_overnight_times()}} in +every trip, so the returned GTFS has them fixed (with \code{by_reference = TRUE}, +even if an error is then raised). + \code{from} and \code{to} are compared with the \code{departure_time} at \code{first_stop} as -written in \code{stop_times} (template times, for trips listed in +written in \code{stop_times}, once fixed (template times, for trips listed in \code{frequencies}), so use times past \code{"24:00:00"} for departures after midnight; trips whose departure there is blank are then left unchanged. Existing \verb{_secs} columns in \code{stop_times} are updated to match. diff --git a/tests/testthat/setup-overnight_feed.R b/tests/testthat/setup-overnight_feed.R new file mode 100644 index 00000000..04a33d50 --- /dev/null +++ b/tests/testthat/setup-overnight_feed.R @@ -0,0 +1,38 @@ +# creates a feed whose times after midnight are written as early morning +# times: trip W runs from 23:50 to "00:35:00", and the frequency-based trip F +# departs every 30 minutes from 23:00 to "01:00:00". this is a setup file, not +# a helper, for the reason given in setup-read_gtfs.R + +overnight_feed <- function() { + feed <- list( + trips = data.table::data.table( + trip_id = c("W", "F"), + route_id = c("R1", "R2"), + service_id = "S1" + ), + stops = data.table::data.table( + stop_id = c("A", "B", "C", "D"), + stop_lat = 0, + stop_lon = c(0, 0.01, 0.02, 0.03) + ), + stop_times = data.table::data.table( + trip_id = c("W", "W", "W", "W", "F", "F"), + stop_id = c("A", "B", "C", "D", "A", "B"), + stop_sequence = c(1:4, 1:2), + arrival_time = c( + "23:50:00", "00:05:00", "00:20:00", "00:35:00", "00:00:00", "00:10:00" + ), + departure_time = c( + "23:50:00", "00:05:00", "00:20:00", "00:35:00", "00:00:00", "00:10:00" + ) + ), + frequencies = data.table::data.table( + trip_id = "F", + start_time = "23:00:00", + end_time = "01:00:00", + headway_secs = 1800L + ) + ) + + return(gtfsio::new_gtfs(feed, "dt_gtfs")) +} diff --git a/tests/testthat/test-build_graph.R b/tests/testthat/test-build_graph.R index 370ce78b..b2555cbc 100644 --- a/tests/testthat/test-build_graph.R +++ b/tests/testthat/test-build_graph.R @@ -372,13 +372,14 @@ test_that("handles times after midnight and decreasing times", { gtfs <- new_feed( stops = new_stops(c("A", "B"), c(0, 1000)), trips = data.table::data.table( - trip_id = c("P1", "W1"), - route_id = c("R1", "R2"), + trip_id = c("P1", "W1", "D1"), + route_id = c("R1", "R2", "R3"), direction_id = 0L ), stop_times = rbind( new_stop_times("P1", c("A", "B"), c("24:30:00", "24:35:00")), - new_stop_times("W1", c("A", "B"), c("23:58:00", "00:02:00")) + new_stop_times("W1", c("A", "B"), c("23:58:00", "00:02:00")), + new_stop_times("D1", c("A", "B"), c("08:02:00", "07:58:00")) ) ) @@ -387,17 +388,41 @@ test_that("handles times after midnight and decreasing times", { expect_identical(in_vehicle$route_id, "R1") expect_equal(in_vehicle$travel_time, 5) - # W1 wrongly goes back to 00:02 after midnight, so it departs at its - # earliest time and its segment has no travel time + # W1 is written going back to 00:02 after midnight, which is fixed to + # 24:02 first, so it departs at 23:58 and runs at night + + night_graph <- tester(gtfs, from = "23:00:00", to = "24:00:00") + in_vehicle <- night_graph$link[link_type == "in_vehicle"] + expect_identical(in_vehicle$route_id, "R2") + expect_equal(in_vehicle$travel_time, 4) + + early_graph <- tester(gtfs, from = "00:00:00", to = "01:00:00") + expect_false("R2" %chin% early_graph$link$route_id) + + # D1 goes back by less than 12 hours, so it departs at its earliest time + # and its segment has no travel time expect_warning( - early_graph <- tester(gtfs, from = "00:00:00", to = "01:00:00"), + day_graph <- tester(gtfs, from = "07:00:00", to = "09:00:00"), class = "gtfstools_links_without_travel_time" ) - in_vehicle <- early_graph$link[link_type == "in_vehicle"] - expect_identical(in_vehicle$route_id, "R2") + in_vehicle <- day_graph$link[link_type == "in_vehicle"] + expect_identical(in_vehicle$route_id, "R3") expect_identical(in_vehicle$travel_time, NA_real_) expect_identical(in_vehicle$free_speed, NA_real_) + + # in 'frequencies', an end time earlier than the start time is fixed too, + # so P1 departs every 10 minutes from 23:00:00 until 25:00:00 + + gtfs$frequencies <- data.table::data.table( + trip_id = "P1", + start_time = "23:00:00", + end_time = "01:00:00", + headway_secs = 600L + ) + night_graph <- tester(gtfs, from = "23:00:00", to = "24:00:00") + in_vehicle <- night_graph$link[link_type == "in_vehicle" & route_id == "R1"] + expect_identical(in_vehicle$departures, 6L) }) test_that("uses the first entry of duplicated trip_ids", { diff --git a/tests/testthat/test-filter_by_time_of_day.R b/tests/testthat/test-filter_by_time_of_day.R index 376e35d7..5de6c716 100644 --- a/tests/testthat/test-filter_by_time_of_day.R +++ b/tests/testthat/test-filter_by_time_of_day.R @@ -531,3 +531,21 @@ test_that("handles trips with some non specified departure/arrival times", { data.table::setindex(gtfs$stop_times, NULL) expect_identical(filtered_stop_times, gtfs$stop_times) }) + +test_that("fixes times after midnight written as early morning times", { + gtfs <- overnight_feed() + original_stop_times <- data.table::copy(gtfs$stop_times) + + result <- filter_by_time_of_day(gtfs, "23:00:00", "25:00:00") + expect_identical( + result$stop_times[trip_id == "W"]$arrival_time, + c("23:50:00", "24:05:00", "24:20:00", "24:35:00") + ) + expect_identical(result$frequencies$end_time, "25:00:00") + + # W's stops after midnight are not within these times of day + + result <- filter_by_time_of_day(gtfs, "00:00:00", "01:00:00") + expect_false("W" %chin% result$stop_times$trip_id) + expect_identical(gtfs$stop_times, original_stop_times) +}) diff --git a/tests/testthat/test-fix_gtfs.R b/tests/testthat/test-fix_gtfs.R index a46cab0e..f2415fbe 100644 --- a/tests/testthat/test-fix_gtfs.R +++ b/tests/testthat/test-fix_gtfs.R @@ -192,10 +192,9 @@ test_that("fills shape_dist_traveled, only in feeds with shapes", { expect_false("shape_dist_traveled" %chin% names(result$stop_times)) }) -test_that("doesn't make up times for trips that end before they start", { - # some trips of this feed have times after midnight written as "00:xx:xx", - # so their times are not interpolated nor capped. only their first and last - # stops have times, so no minimum dwell time is set either +test_that("fixes the times of trips that run past midnight", { + # some trips of this feed have times after midnight written as "00:xx:xx". + # they are fixed first, so their times are then capped and interpolated poa_gtfs <- read_gtfs( system.file("extdata/poa_gtfs.zip", package = "gtfstools") @@ -210,20 +209,25 @@ test_that("doesn't make up times for trips that end before they start", { invokeRestart("muffleWarning") } ) - expect_true("gtfstools_backwards_times" %in% warning_classes) + expect_false("gtfstools_backwards_times" %in% warning_classes) - # shape_dist_traveled is added to every trip, so only the original columns - # are compared + # the trip keeps its start time and ends after midnight. its intermediate + # times are filled, and depend on the capped speeds, so only their order is + # checked - original_cols <- names(poa_gtfs$stop_times) - expect_identical( - result$stop_times[trip_id == trip, .SD, .SDcols = original_cols], - poa_gtfs$stop_times[trip_id == trip] - ) - expect_identical( - setdiff(names(result$stop_times), original_cols), - "shape_dist_traveled" + trip_times <- result$stop_times[trip_id == trip][order(stop_sequence)] + expect_identical(trip_times$arrival_time[1], "23:10:00") + expect_true(all(trip_times$arrival_time != "")) + expect_true(all(trip_times$departure_time != "")) + + secs <- as.vector( + rbind( + string_to_seconds(trip_times$arrival_time), + string_to_seconds(trip_times$departure_time) + ) ) + expect_false(is.unsorted(secs)) + expect_gte(secs[length(secs)], string_to_seconds("24:02:00")) }) test_that("unit arguments work correctly", { diff --git a/tests/testthat/test-fix_overnight_times.R b/tests/testthat/test-fix_overnight_times.R new file mode 100644 index 00000000..339f6463 --- /dev/null +++ b/tests/testthat/test-fix_overnight_times.R @@ -0,0 +1,489 @@ +spo_path <- system.file("extdata/spo_gtfs.zip", package = "gtfstools") +poa_path <- system.file("extdata/poa_gtfs.zip", package = "gtfstools") + +tester <- function(gtfs = wrap_gtfs) fix_overnight_times(gtfs) + +# creates a feed with one trip per element of 'times', a named list with the +# arrival and departure times of each trip (departures default to arrivals) + +synthetic_feed <- function(times) { + stop_times <- data.table::rbindlist( + lapply( + names(times), + function(id) { + arr <- times[[id]]$arr + dep <- times[[id]]$dep + if (is.null(dep)) dep <- arr + data.table::data.table( + trip_id = id, + arrival_time = arr, + departure_time = dep, + stop_id = paste0("s", seq_along(arr)), + stop_sequence = seq_along(arr) + ) + } + ) + ) + + feed <- list( + trips = data.table::data.table(trip_id = names(times), route_id = "r1"), + stop_times = stop_times + ) + + return(gtfsio::new_gtfs(feed, "dt_gtfs")) +} + +# trips that run past midnight, with the times they should have after the fix + +wrap_times <- list( + arrival = list(arr = c("23:00:00", "23:50:00", "00:10:00")), + arr_dep = list( + arr = c("23:00:00", "23:59:00", "00:20:00"), + dep = c("23:00:00", "00:01:00", "00:20:00") + ), + one_time = list( + arr = c("23:00:00", "", "00:10:00", ""), + dep = c("23:00:00", "", "", "00:20:00") + ), + leading_blank = list(arr = c("", "23:30:00", "", "00:30:00")), + mixed_after = list(arr = c("24:30:00", "00:40:00", "24:50:00")), + forward_gap = list(arr = c("00:10:00", "24:20:00")), + double_wrap = list( + arr = c( + "22:00:00", "00:30:00", "08:00:00", "16:00:00", "23:50:00", "00:10:00" + ) + ), + correct = list(arr = c("23:00:00", "24:30:00", "25:10:00")), + long_trip = list(arr = c("06:00:00", "", "19:30:00")), + daytime = list(arr = c("08:00:00", "08:30:00", "09:00:00")) +) + +expected_times <- list( + arrival = list(arr = c("23:00:00", "23:50:00", "24:10:00")), + arr_dep = list( + arr = c("23:00:00", "23:59:00", "24:20:00"), + dep = c("23:00:00", "24:01:00", "24:20:00") + ), + one_time = list( + arr = c("23:00:00", "", "24:10:00", ""), + dep = c("23:00:00", "", "", "24:20:00") + ), + leading_blank = list(arr = c("", "23:30:00", "", "24:30:00")), + mixed_after = list(arr = c("24:30:00", "24:40:00", "24:50:00")), + double_wrap = list( + arr = c( + "22:00:00", "24:30:00", "32:00:00", "40:00:00", "47:50:00", "48:10:00" + ) + ), + forward_gap = wrap_times$forward_gap, + correct = wrap_times$correct, + long_trip = wrap_times$long_trip, + daytime = wrap_times$daytime +) + +wrap_gtfs <- synthetic_feed(wrap_times) +wrap_gtfs$frequencies <- data.table::data.table( + trip_id = "correct", + start_time = c("23:00:00", "05:00:00", "22:00:00", "48:30:00", "25:00:00"), + end_time = c("01:00:00", "06:00:00", "24:30:00", "00:10:00", "01:00:00"), + headway_secs = 600L +) + +# trips whose times still go back after the fix, which raise a warning + +warn_gtfs <- synthetic_feed( + list( + small_back = list(arr = c("08:00:00", "08:05:00", "08:04:00")), + exactly_12h = list(arr = c("20:00:00", "08:00:00")), + back_after_fix = list(arr = c("24:30:00", "00:20:00")) + ) +) + +trip_times <- function(gtfs, id) { + st <- gtfs$stop_times[trip_id == id] + return(list(arr = st$arrival_time, dep = st$departure_time)) +} + +expect_trip_times <- function(gtfs, id, expected) { + if (is.null(expected$dep)) expected$dep <- expected$arr + expect_identical(trip_times(gtfs, id), expected[c("arr", "dep")]) +} + +# tests --- + +test_that("raises error due to incorrect input types", { + expect_error(tester(unclass(wrap_gtfs))) + + no_arrival <- copy_gtfs_without_field(wrap_gtfs, "stop_times", "arrival_time") + expect_error(tester(no_arrival)) + + bad_sequence <- copy_gtfs_diff_field_class( + wrap_gtfs, + "stop_times", + "stop_sequence", + "character" + ) + expect_error(tester(bad_sequence)) + + bad_end <- copy_gtfs_diff_field_class( + wrap_gtfs, + "frequencies", + "end_time", + "factor" + ) + expect_error(tester(bad_end)) + + expect_error(tester(copy_gtfs_without_file(wrap_gtfs, "stop_times"))) +}) + +test_that("results in a dt_gtfs object", { + result <- tester() + expect_s3_class(result, c("dt_gtfs", "gtfs", "list")) + for (table in names(result)) { + expect_s3_class(result[[table]], "data.table") + } +}) + +test_that("moves the times after midnight one day later", { + result <- expect_silent(tester()) + + for (id in names(expected_times)) { + expect_trip_times(result, id, expected_times[[id]]) + } + + # only the time columns change + + expect_identical( + result$stop_times[, .(trip_id, stop_id, stop_sequence)], + wrap_gtfs$stop_times[, .(trip_id, stop_id, stop_sequence)] + ) + expect_identical(result$trips, wrap_gtfs$trips) +}) + +test_that("leaves rows without trip_id or stop_sequence unchanged", { + gtfs <- wrap_gtfs + gtfs$stop_times <- rbind( + wrap_gtfs$stop_times, + data.table::data.table( + trip_id = c(NA, "arrival"), + arrival_time = "00:05:00", + departure_time = "00:05:00", + stop_id = "s1", + stop_sequence = c(10L, NA) + ) + ) + + result <- tester(gtfs) + expect_identical( + result$stop_times[is.na(trip_id) | is.na(stop_sequence), arrival_time], + c("00:05:00", "00:05:00") + ) + expect_trip_times( + result, + "arrival", + list(arr = c("23:00:00", "23:50:00", "24:10:00", "00:05:00")) + ) +}) + +test_that("warns about times that still go back", { + expect_warning( + result <- tester(warn_gtfs), + class = "gtfstools_backwards_times" + ) + + expect_trip_times( + result, + "small_back", + list(arr = c("08:00:00", "08:05:00", "08:04:00")) + ) + expect_trip_times( + result, + "exactly_12h", + list(arr = c("20:00:00", "08:00:00")) + ) + expect_trip_times( + result, + "back_after_fix", + list(arr = c("24:30:00", "24:20:00")) + ) + + # every trip is named in the warning + + warning_msg <- tryCatch( + tester(warn_gtfs), + warning = function(w) conditionMessage(w) + ) + for (id in c("small_back", "exactly_12h", "back_after_fix")) { + expect_match(warning_msg, id, fixed = TRUE) + } +}) + +test_that("fixes end times earlier than start times in frequencies", { + result <- tester() + expect_identical( + result$frequencies$end_time, + c("25:00:00", "06:00:00", "24:30:00", "72:10:00", "49:00:00") + ) + expect_identical( + result$frequencies$start_time, + wrap_gtfs$frequencies$start_time + ) + + no_frequencies <- copy_gtfs_without_file(wrap_gtfs, "frequencies") + result <- tester(no_frequencies) + expect_null(result$frequencies) + expect_trip_times(result, "arrival", expected_times$arrival) +}) + +test_that("updates existing _secs columns", { + gtfs <- convert_time_to_seconds(wrap_gtfs) + result <- tester(gtfs) + + expect_identical( + result$stop_times$arrival_time_secs, + string_to_seconds(result$stop_times$arrival_time) + ) + expect_identical( + result$stop_times$departure_time_secs, + string_to_seconds(result$stop_times$departure_time) + ) + expect_identical( + result$frequencies$end_time_secs, + string_to_seconds(result$frequencies$end_time) + ) +}) + +test_that("raises an error when a fixed time is out of range", { + gtfs <- synthetic_feed(list(late = list(arr = c("9999:00:00", "00:10:00")))) + expect_error(tester(gtfs), class = "gtfstools_time_out_of_range") + + gtfs <- wrap_gtfs + gtfs$frequencies <- data.table::data.table( + trip_id = "correct", + start_time = "9999:00:00", + end_time = "9990:00:00", + headway_secs = 600L + ) + expect_error(tester(gtfs), class = "gtfstools_time_out_of_range") +}) + +test_that("doesn't change given gtfs", { + original_gtfs <- gtfsio::new_gtfs( + lapply(wrap_gtfs, data.table::copy), + "dt_gtfs" + ) + gtfs <- gtfsio::new_gtfs(lapply(wrap_gtfs, data.table::copy), "dt_gtfs") + expect_identical(original_gtfs, gtfs) + + result <- tester(gtfs) + expect_false(identical(result$stop_times, gtfs$stop_times)) + expect_false(identical(result$frequencies, gtfs$frequencies)) + expect_equal(original_gtfs, gtfs, ignore_attr = TRUE) + + original_gtfs <- read_gtfs(poa_path) + gtfs <- read_gtfs(poa_path) + expect_identical(original_gtfs, gtfs) + + result <- tester(gtfs) + expect_false(identical(result$stop_times, gtfs$stop_times)) + expect_equal(original_gtfs, gtfs, ignore_attr = TRUE) +}) + +test_that("returns feeds without times after midnight unchanged", { + spo_gtfs <- read_gtfs(spo_path) + result <- expect_silent(tester(spo_gtfs)) + expect_identical(result, spo_gtfs) +}) + +test_that("fixes the trips of poa that run past midnight", { + poa_gtfs <- read_gtfs(poa_path) + result <- expect_silent(tester(poa_gtfs)) + + changed <- result$stop_times$arrival_time != poa_gtfs$stop_times$arrival_time + expect_length(unique(result$stop_times$trip_id[changed]), 10) + + expect_trip_times( + result, + "T2-1@1#2310", + list( + arr = c("23:10:00", rep("", 60), "24:02:00"), + dep = c("23:10:00", rep("", 60), "24:02:00") + ) + ) +}) + +test_that("the internal fix can be restricted to some trips", { + result <- fix_overnight_stop_times(wrap_gtfs, trip_id = "arrival") + expect_trip_times(result, "arrival", expected_times$arrival) + expect_trip_times(result, "arr_dep", wrap_times$arr_dep) + + # with shuffled rows and an unknown id, the selected trips are fixed as when + # every trip is fixed, and the others are left unchanged + + shuffled_gtfs <- wrap_gtfs + shuffled_gtfs$stop_times <- wrap_gtfs$stop_times[ + rev(seq_len(nrow(wrap_gtfs$stop_times))) + ] + selected <- c("one_time", "double_wrap", "unknown") + result <- fix_overnight_stop_times(shuffled_gtfs, trip_id = selected) + for (id in c("one_time", "double_wrap")) { + expect_trip_times(result, id, lapply(expected_times[[id]], rev)) + } + expect_identical( + result$stop_times[!trip_id %chin% selected], + shuffled_gtfs$stop_times[!trip_id %chin% selected] + ) + + expect_identical( + fix_overnight_stop_times(wrap_gtfs, trip_id = character(0)), + wrap_gtfs + ) +}) + +test_that("nested calls don't fix the times again", { + poa_gtfs <- read_gtfs(poa_path) + + # only the runs that change some time are counted, which is checked before + # each run, since tables fixed by reference look the same afterwards + + n_fixes <- 0 + real_fix <- fix_overnight_stop_times + local_mocked_bindings( + fix_overnight_stop_times = function( + gtfs, + trip_id = NULL, + warn = TRUE, + by_reference = FALSE, + detect_only = FALSE + ) { + if (!detect_only) { + changes <- real_fix(gtfs, trip_id, warn = FALSE, detect_only = TRUE) + if (isTRUE(changes)) n_fixes <<- n_fixes + 1 + } + real_fix(gtfs, trip_id, warn, by_reference, detect_only) + } + ) + count_fixes <- function(expr) { + n_fixes <<- 0 + force(expr) + return(n_fixes) + } + + expect_identical(count_fixes(fix_gtfs(poa_gtfs)), 1) + expect_identical( + count_fixes(get_trip_speed(poa_gtfs, from = "07:00:00", to = "09:00:00")), + 1 + ) + expect_identical( + count_fixes( + suppressWarnings( + build_graph(poa_gtfs, from = "07:00:00", to = "08:00:00") + ) + ), + 1 + ) + expect_identical( + count_fixes( + get_stop_frequency(poa_gtfs, from = "07:00:00", to = "08:00:00") + ), + 1 + ) + if (report_deps_available()) { + expect_identical( + count_fixes( + get_summary( + poa_gtfs, + output_path = tempfile("get_summary"), + html_preview = FALSE + ) + ), + 1 + ) + } + + by_reference_gtfs <- read_gtfs(poa_path) + expect_identical( + count_fixes(set_dwell_time(by_reference_gtfs, 30, by_reference = TRUE)), + 1 + ) + + # a function called by the user while a call is in progress (e.g. in a + # calling handler) fixes the times of its own feed + + gtfs <- overnight_feed() + duration <- NULL + withCallingHandlers( + get_trip_duration(gtfs, c("W", "unknown")), + warning = function(w) { + duration <<- get_trip_duration(gtfs, "W")$duration + invokeRestart("muffleWarning") + } + ) + expect_equal(duration, 45) + + gtfs <- overnight_feed() + gtfs$frequencies[, `:=`(start_time = "9999:00:00", end_time = "9990:00:00")] + expect_error( + get_route_frequency(gtfs, from = "23:00:00", to = "24:00:00"), + class = "gtfstools_time_out_of_range" + ) +}) + +test_that("has_overnight_trips() detects the times that would be fixed", { + expect_true(has_overnight_trips(read_gtfs(poa_path))) + expect_true(has_overnight_trips(wrap_gtfs)) + expect_true(has_overnight_trips(overnight_feed())) + + # feeds without them, or whose times were already fixed + + expect_false(has_overnight_trips(read_gtfs(spo_path))) + expect_false(has_overnight_trips(fix_overnight_times(read_gtfs(poa_path)))) + expect_false(has_overnight_trips(fix_overnight_times(overnight_feed()))) + + # times that go back by 12 hours or less are not fixed, so they don't count + + small_steps <- synthetic_feed( + list( + small_back = list(arr = c("08:00:00", "08:05:00", "08:04:00")), + exactly_12h = list(arr = c("20:00:00", "08:00:00")) + ) + ) + expect_silent(result <- has_overnight_trips(small_steps)) + expect_false(result) + + # only frequencies entries that end before they start + + gtfs <- fix_overnight_times(overnight_feed()) + gtfs$frequencies$end_time <- "01:00:00" + expect_true(has_overnight_trips(gtfs)) + + # tables that can't be checked are skipped + + no_sequence <- copy_gtfs_without_field( + overnight_feed(), + "stop_times", + "stop_sequence" + ) + no_sequence$frequencies <- NULL + expect_false(has_overnight_trips(no_sequence)) +}) + +test_that("warns about times that go back in feeds without times to fix", { + # the feed has no times after midnight to fix, but its times that go back + # by 12 hours or less, and its malformed times, are still reported + + small_back <- synthetic_feed( + list(small_back = list(arr = c("08:00:00", "08:05:00", "08:04:00"))) + ) + expect_false(has_overnight_trips(small_back)) + expect_warning( + result <- tester(small_back), + class = "gtfstools_backwards_times" + ) + expect_identical(result$stop_times, small_back$stop_times) + + malformed <- synthetic_feed( + list(malformed = list(arr = c("08:00:00", "ab:cd", "08:10:00"))) + ) + expect_warning(tester(malformed), class = "gtfstools_malformed_time") +}) diff --git a/tests/testthat/test-get_dwell_time.R b/tests/testthat/test-get_dwell_time.R index 11373ba6..f0e56c83 100644 --- a/tests/testthat/test-get_dwell_time.R +++ b/tests/testthat/test-get_dwell_time.R @@ -220,3 +220,38 @@ test_that("only returns the visits that arrive within the time of day", { expect_identical(nrow(dwell), 0L) expect_vector(dwell$dwell_time, numeric(0)) }) + +test_that("fixes times after midnight written as early morning times", { + # a trip whose times after midnight are written as early morning times, + # including the departure from its second stop + + overnight_gtfs <- gtfsio::new_gtfs( + list( + stop_times = data.table::data.table( + trip_id = "t1", + stop_id = c("s1", "s2", "s3"), + stop_sequence = 1:3, + arrival_time = c("23:50:00", "23:58:00", "00:05:00"), + departure_time = c("23:50:00", "00:01:00", "00:05:00") + ) + ), + "dt_gtfs" + ) + + original_gtfs <- gtfsio::new_gtfs( + lapply(overnight_gtfs, data.table::copy), + "dt_gtfs" + ) + + dwell <- tester(overnight_gtfs) + expect_equal(dwell$dwell_time, c(0, 180, 0)) + expect_equal(overnight_gtfs, original_gtfs, ignore_attr = TRUE) + + # the visits after midnight are compared with the time of day as fixed + + dwell <- get_dwell_time(overnight_gtfs, from = "00:00:00", to = "01:00:00") + expect_identical(nrow(dwell), 0L) + + dwell <- get_dwell_time(overnight_gtfs, from = "24:00:00", to = "25:00:00") + expect_identical(dwell$stop_id, "s3") +}) diff --git a/tests/testthat/test-get_route_frequency.R b/tests/testthat/test-get_route_frequency.R index 30c3e426..16edfc65 100644 --- a/tests/testthat/test-get_route_frequency.R +++ b/tests/testthat/test-get_route_frequency.R @@ -316,3 +316,39 @@ test_that("doesn't change given gtfs", { frequency <- tester(gtfs) expect_equal(original_gtfs, gtfs, ignore_attr = TRUE) }) + +test_that("fixes times after midnight written as early morning times", { + # W departs at 23:50, and F every 30 minutes from 23:00 to "01:00:00" + + gtfs <- overnight_feed() + original_stop_times <- data.table::copy(gtfs$stop_times) + + result <- get_route_frequency(gtfs, from = "23:00:00", to = "24:00:00") + expect_identical(result$departures, c(1L, 2L)) + result <- get_route_frequency(gtfs, from = "24:00:00", to = "25:00:00") + expect_identical(result$route_id, "R2") + expect_identical(result$departures, 2L) + result <- get_route_frequency(gtfs, from = "00:00:00", to = "01:00:00") + expect_identical(nrow(result), 0L) + expect_identical(gtfs$stop_times, original_stop_times) + + # poa's trip departs at 23:10, not at its last time written as "00:02:00" + + poa_gtfs <- read_gtfs( + system.file("extdata/poa_gtfs.zip", package = "gtfstools") + ) + late <- get_route_frequency( + poa_gtfs, + "T2", + from = "23:00:00", + to = "23:59:59" + ) + early <- get_route_frequency( + poa_gtfs, + "T2", + from = "00:00:00", + to = "01:00:00" + ) + expect_gt(sum(late$departures), 0L) + expect_identical(sum(early$departures), 0L) +}) diff --git a/tests/testthat/test-get_stop_frequency.R b/tests/testthat/test-get_stop_frequency.R index 3a264356..33e6e965 100644 --- a/tests/testthat/test-get_stop_frequency.R +++ b/tests/testthat/test-get_stop_frequency.R @@ -442,3 +442,33 @@ test_that("doesn't change given gtfs", { frequency <- tester(gtfs, stop_id = "18848") expect_equal(original_gtfs, gtfs, ignore_attr = TRUE) }) + +test_that("fixes times after midnight written as early morning times", { + # after midnight, F departs from A at 24:00 and 24:30, and W from B at 24:05 + # and C at 24:20 (D is W's last stop, and B is F's) + + gtfs <- overnight_feed() + original_stop_times <- data.table::copy(gtfs$stop_times) + + result <- get_stop_frequency(gtfs, from = "24:00:00", to = "25:00:00") + expect_identical(result$stop_id, c("A", "B", "C")) + expect_identical(result$departures, c(2L, 1L, 1L)) + + result <- get_stop_frequency( + gtfs, + stop_id = "B", + from = "24:00:00", + to = "25:00:00" + ) + expect_identical(result$departures, 1L) + + result <- get_stop_frequency(gtfs, from = "00:00:00", to = "01:00:00") + expect_identical(nrow(result), 0L) + expect_identical(gtfs$stop_times, original_stop_times) + + # without arrival_time, the times can't be fixed and are used as written + + no_arrival <- copy_gtfs_without_field(gtfs, "stop_times", "arrival_time") + result <- get_stop_frequency(no_arrival, from = "00:00:00", to = "01:00:00") + expect_identical(result$stop_id, c("B", "C")) +}) diff --git a/tests/testthat/test-get_trip_duration.R b/tests/testthat/test-get_trip_duration.R index 47b884ea..6cef696c 100644 --- a/tests/testthat/test-get_trip_duration.R +++ b/tests/testthat/test-get_trip_duration.R @@ -110,9 +110,13 @@ test_that("calculates duration correctly", { modified_st_gtfs <- gtfs modified_st_gtfs$stop_times <- data.table::copy(gtfs$stop_times) - modified_st_gtfs$stop_times[ + last_stop <- modified_st_gtfs$stop_times[ trip_id == "CPTM L07-0", - arrival_time := "24:00:00" + max(stop_sequence) + ] + modified_st_gtfs$stop_times[ + trip_id == "CPTM L07-0" & stop_sequence == last_stop, + `:=`(arrival_time = "24:00:00", departure_time = "24:00:00") ] durations <- tester(modified_st_gtfs, "CPTM L07-0") @@ -305,3 +309,49 @@ test_that("empty results have the same duration type as non-empty ones", { } } }) + +test_that("fixes times after midnight written as early morning times", { + # a trip whose times after midnight are written as early morning times, + # including the departure from its second stop + + overnight_gtfs <- gtfsio::new_gtfs( + list( + stop_times = data.table::data.table( + trip_id = "t1", + stop_id = c("s1", "s2", "s3"), + stop_sequence = 1:3, + arrival_time = c("23:50:00", "23:58:00", "00:05:00"), + departure_time = c("23:50:00", "00:01:00", "00:05:00") + ) + ), + "dt_gtfs" + ) + + original_gtfs <- gtfsio::new_gtfs( + lapply(overnight_gtfs, data.table::copy), + "dt_gtfs" + ) + + expect_no_warning( + durations <- tester(overnight_gtfs), + class = "gtfstools_backwards_times" + ) + expect_equal(durations$duration, 15) + expect_equal(overnight_gtfs, original_gtfs, ignore_attr = TRUE) + + poa_gtfs <- read_gtfs( + system.file("extdata/poa_gtfs.zip", package = "gtfstools") + ) + expect_equal(get_trip_duration(poa_gtfs, "T2-1@1#2310")$duration, 52) + + # without stop_sequence the times can't be ordered, so they are used as + # written + + no_sequence <- copy_gtfs_without_field( + overnight_gtfs, + "stop_times", + "stop_sequence" + ) + durations <- get_trip_duration(no_sequence) + expect_equal(durations$duration, 1437) +}) diff --git a/tests/testthat/test-get_trip_length.R b/tests/testthat/test-get_trip_length.R index 95d26c3d..01649eff 100644 --- a/tests/testthat/test-get_trip_length.R +++ b/tests/testthat/test-get_trip_length.R @@ -759,3 +759,32 @@ test_that("from and to select single-stop trips by their only stop", { result <- get_trip_length(one_stop_gtfs, from = "12:00:00") expect_false("CPTM L07-0" %chin% result$trip_id) }) + +test_that("selects segments after midnight from the fixed times", { + # W's segments from B (24:05) and C (24:20) depart after midnight + + gtfs <- overnight_feed() + lengths <- get_trip_length( + gtfs, + by = "segment", + method = "euclidean", + from = "24:00:00", + to = "25:00:00" + ) + durations <- get_trip_segment_duration( + gtfs, + from = "24:00:00", + to = "25:00:00" + ) + expect_identical(lengths$segment, c(2L, 3L)) + expect_identical(lengths$segment, durations$segment) + + lengths <- get_trip_length( + gtfs, + by = "segment", + method = "euclidean", + from = "00:00:00", + to = "01:00:00" + ) + expect_false("W" %chin% lengths$trip_id) +}) diff --git a/tests/testthat/test-get_trip_segment_duration.R b/tests/testthat/test-get_trip_segment_duration.R index ed750d2c..c7f404bc 100644 --- a/tests/testthat/test-get_trip_segment_duration.R +++ b/tests/testthat/test-get_trip_segment_duration.R @@ -260,3 +260,33 @@ test_that("returns a typed empty table when stop_times is empty", { expect_type(tester(empty_gtfs)$duration, "double") }) + +test_that("fixes times after midnight written as early morning times", { + # a trip whose times after midnight are written as early morning times, + # including the departure from its second stop + + overnight_gtfs <- gtfsio::new_gtfs( + list( + stop_times = data.table::data.table( + trip_id = "t1", + stop_id = c("s1", "s2", "s3"), + stop_sequence = 1:3, + arrival_time = c("23:50:00", "23:58:00", "00:05:00"), + departure_time = c("23:50:00", "00:01:00", "00:05:00") + ) + ), + "dt_gtfs" + ) + + original_gtfs <- gtfsio::new_gtfs( + lapply(overnight_gtfs, data.table::copy), + "dt_gtfs" + ) + + expect_no_warning( + durations <- tester(overnight_gtfs), + class = "gtfstools_backwards_times" + ) + expect_equal(durations$duration, c(8, 4)) + expect_equal(overnight_gtfs, original_gtfs, ignore_attr = TRUE) +}) diff --git a/tests/testthat/test-get_trip_speed.R b/tests/testthat/test-get_trip_speed.R index 12f74887..4a1e5f14 100644 --- a/tests/testthat/test-get_trip_speed.R +++ b/tests/testthat/test-get_trip_speed.R @@ -314,3 +314,15 @@ test_that("from and to raise the warnings only once", { expect_length(warnings, 1) } }) + +test_that("fixes times after midnight written as early morning times", { + poa_gtfs <- read_gtfs( + system.file("extdata/poa_gtfs.zip", package = "gtfstools") + ) + + # this trip runs from 23:10:00 to "00:02:00" (i.e. 52 minutes) + + speed <- get_trip_speed(poa_gtfs, "T2-1@1#2310") + trip_length <- get_trip_length(poa_gtfs, "T2-1@1#2310") + expect_equal(speed$speed, trip_length$length / (52 / 60)) +}) diff --git a/tests/testthat/test-interpolate_stop_times.R b/tests/testthat/test-interpolate_stop_times.R index 289c3978..694a8c13 100644 --- a/tests/testthat/test-interpolate_stop_times.R +++ b/tests/testthat/test-interpolate_stop_times.R @@ -133,6 +133,17 @@ test_that("handles times past midnight", { feed <- synthetic_feed(c(0, 0.01, 0.02), c("25:59:00", "", "26:01:00")) result <- tester(feed, method = "euclidean") expect_identical(result$stop_times$arrival_time[2], "26:00:00") + + # times after midnight written as early morning times are fixed first + + feed <- synthetic_feed(c(0, 0.01, 0.02), c("23:59:00", "", "00:01:00")) + original_stop_times <- data.table::copy(feed$stop_times) + expect_no_warning(result <- tester(feed, method = "euclidean")) + expect_identical(feed$stop_times, original_stop_times) + expect_identical( + result$stop_times$arrival_time, + c("23:59:00", "24:00:00", "24:01:00") + ) }) test_that("spreads times evenly if the distance between timepoints is 0", { @@ -250,10 +261,10 @@ test_that("falls back to euclidean distances without shapes", { }) test_that("leaves spans whose arrival is earlier than the departure blank", { - feed <- synthetic_feed(c(0, 0.01, 0.02), c("23:59:00", "", "00:01:00")) + feed <- synthetic_feed(c(0, 0.01, 0.02), c("08:59:00", "", "08:57:00")) expect_warning( result <- tester(feed, method = "euclidean"), - regexp = "after midnight", + regexp = "earlier than the previous departure", class = "gtfstools_times_not_interpolated" ) expect_identical(result$stop_times, feed$stop_times) @@ -261,14 +272,17 @@ test_that("leaves spans whose arrival is earlier than the departure blank", { # a trip with a backwards span and other blanks is listed for both reasons feed <- synthetic_feed( c(0, 0.01, 0.02, 0.03), - c("23:59:00", "", "00:01:00", "") + c("08:59:00", "", "08:57:00", "") ) warning <- expect_warning( tester(feed, method = "euclidean"), class = "gtfstools_times_not_interpolated" ) expect_match(conditionMessage(warning), "Affected trip") - expect_match(conditionMessage(warning), "after midnight") + expect_match( + conditionMessage(warning), + "earlier than the previous departure" + ) }) test_that("doesn't mix up stops of different trips", { @@ -521,12 +535,12 @@ test_that("drops dwell times that exceed the time between timepoints", { }) test_that("overwrite = TRUE keeps the times of trips it can't interpolate", { - # t1's last stop is written as "00:05:00" instead of "24:05:00". t2 can't - # be interpolated either, but its intermediate stop was already blank + # t1's last stop is earlier than its first one. t2 can't be interpolated + # either, but its intermediate stop was already blank feed <- synthetic_feed( c(0, 0.01, 0.02), - arrival_time = c("23:50:00", "", "00:05:00"), - departure_time = c("23:50:00", "23:55:00", "00:05:00") + arrival_time = c("08:50:00", "", "08:45:00"), + departure_time = c("08:50:00", "08:55:00", "08:45:00") ) t2 <- data.table::copy(feed$stop_times) t2[, `:=`(trip_id = "t2", arrival_time = c("08:00:00", "", ""))] @@ -599,18 +613,15 @@ test_that("returns feeds without blank times unchanged", { }) test_that("fills poa's blank times, which never decrease", { - expect_warning( + # trips that cross midnight are written with times such as "00:02:00", + # which are fixed first, so every blank time is filled + expect_no_warning( result <- tester(poa_gtfs), class = "gtfstools_times_not_interpolated" ) st <- data.table::copy(result$stop_times) + expect_false(any(st$arrival_time == "" | st$departure_time == "")) - # trips that cross midnight are written with times such as "00:02:00" - st[, is_blank := arrival_time == "" | departure_time == ""] - blank_trips <- unique(st[is_blank == TRUE]$trip_id) - expect_length(blank_trips, 10) - - st <- st[!trip_id %chin% blank_trips] st[, `:=`( arr = string_to_seconds(arrival_time), dep = string_to_seconds(departure_time) @@ -620,11 +631,11 @@ test_that("fills poa's blank times, which never decrease", { expect_true(all(st$dep >= st$arr)) expect_true(all(st$next_arr >= st$dep, na.rm = TRUE)) - # the original timepoints are kept + # the original timepoints are kept, with their times after midnight fixed is_timepoint <- poa_gtfs$stop_times$arrival_time != "" expect_identical( result$stop_times$arrival_time[is_timepoint], - poa_gtfs$stop_times$arrival_time[is_timepoint] + fix_overnight_times(poa_gtfs)$stop_times$arrival_time[is_timepoint] ) }) @@ -634,10 +645,8 @@ test_that("works with stop_times grouped by dplyr (#130)", { grouped_gtfs <- read_gtfs(poa_path) grouped_gtfs$stop_times <- dplyr::group_by(grouped_gtfs$stop_times, trip_id) - # poa_gtfs has trips whose times can't be interpolated, which warn - - result <- suppressWarnings(tester(grouped_gtfs)) + result <- tester(grouped_gtfs) - expect_identical(result, suppressWarnings(tester())) + expect_identical(result, tester()) expect_s3_class(grouped_gtfs$stop_times, "grouped_df") }) diff --git a/tests/testthat/test-set_dwell_time.R b/tests/testthat/test-set_dwell_time.R index 8a5027e6..5efadea0 100644 --- a/tests/testthat/test-set_dwell_time.R +++ b/tests/testthat/test-set_dwell_time.R @@ -307,7 +307,10 @@ test_that("leaves visits with blank times unchanged with a warning", { new_gtfs <- tester(poa_gtfs, trip_id = trip, stop_id = untimed_stop), class = "gtfstools_blank_dwell_time" ) - expect_identical(new_gtfs$stop_times, poa_gtfs$stop_times) + expect_identical( + new_gtfs$stop_times, + fix_overnight_times(poa_gtfs)$stop_times + ) # the warning counts the calls and the trips separately @@ -595,3 +598,53 @@ test_that("route_type restricts the changed visits", { expect_error(set_dwell_time(gtfs, dwell_time = 30, route_type = "2")) }) + +test_that("fixes times after midnight written as early morning times", { + # a trip whose times after midnight are written as early morning times, + # including the departure from its second stop + + overnight_gtfs <- gtfsio::new_gtfs( + list( + stop_times = data.table::data.table( + trip_id = "t1", + stop_id = c("s1", "s2", "s3"), + stop_sequence = 1:3, + arrival_time = c("23:50:00", "23:58:00", "00:05:00"), + departure_time = c("23:50:00", "00:01:00", "00:05:00") + ) + ), + "dt_gtfs" + ) + + # the dwell time at s2 is 3 minutes, from 23:58:00 to "00:01:00" + + result <- set_dwell_time(overnight_gtfs, 120, stop_id = "s2") + expect_identical( + result$stop_times$departure_time, + c("23:50:00", "24:00:00", "24:04:00") + ) + expect_identical( + result$stop_times$arrival_time, + c("23:50:00", "23:58:00", "24:04:00") + ) + + # the time of day of a visit after midnight is compared as fixed + + expect_identical( + set_dwell_time( + overnight_gtfs, + 120, + stop_id = "s2", + from = "00:00:00", + to = "01:00:00" + )$stop_times, + fix_overnight_times(overnight_gtfs)$stop_times + ) + + # by reference, the given feed gets the fixed times and the new dwell time + + expect_invisible( + set_dwell_time(overnight_gtfs, 120, stop_id = "s2", by_reference = TRUE) + ) + expect_identical(overnight_gtfs$stop_times, result$stop_times) +}) diff --git a/tests/testthat/test-set_max_speed.R b/tests/testthat/test-set_max_speed.R index 4b618044..64846484 100644 --- a/tests/testthat/test-set_max_speed.R +++ b/tests/testthat/test-set_max_speed.R @@ -220,7 +220,10 @@ test_that("keeps dwell times and blank times in feeds with blank times", { poa_gtfs <- read_gtfs( system.file("extdata/poa_gtfs.zip", package = "gtfstools") ) - expect_warning( + # its trips with times after midnight written as "00:xx:xx" are fixed + # first, so they are capped too + + expect_no_warning( result <- tester(poa_gtfs, max_speed = 20), class = "gtfstools_backwards_times" ) @@ -317,14 +320,14 @@ test_that("caps spans with zero duration", { }) test_that("leaves spans that end before they start unchanged, with a warning", { - # times after midnight written as "00:xx:xx". the first span is fast, the - # second ends before it starts and the last one is fast + # the first span is fast, the second ends before it starts and the last one + # is fast feed <- synthetic_feed( list( list( - arr = c("23:58:00", "23:59:00", "00:01:00", "00:30:00", "00:31:00"), - dep = c("23:58:00", "23:59:00", "00:01:00", "00:30:00", "00:31:00") + arr = c("08:58:00", "08:59:00", "08:57:00", "09:26:00", "09:27:00"), + dep = c("08:58:00", "08:59:00", "08:57:00", "09:26:00", "09:27:00") ) ), route_id = "r1" @@ -345,10 +348,36 @@ test_that("leaves spans that end before they start unchanged, with a warning", { ) }) +test_that("fixes times after midnight written as early morning times", { + # the same trip, with times after midnight written as "00:xx:xx". they are + # fixed first, so the second span doesn't go back and is capped too + + times <- c("23:58:00", "23:59:00", "00:01:00", "00:30:00", "00:31:00") + feed <- synthetic_feed(list(list(arr = times, dep = times)), route_id = "r1") + + expect_no_warning(result <- tester(feed)) + + fixed <- c("23:58:00", "23:59:00", "24:01:00", "24:30:00", "24:31:00") + fixed_feed <- synthetic_feed( + list(list(arr = fixed, dep = fixed)), + route_id = "r1" + ) + expect_identical(result, tester(fixed_feed)) + expect_false(identical(result, fix_overnight_times(feed))) + + # a time of day with times after midnight written as "00:xx:xx" no longer + # matches the fixed stops + + expect_identical( + tester(feed, first_stop = "s3", from = "00:00:00", to = "01:00:00"), + fix_overnight_times(feed) + ) +}) + test_that("warns only about the selected trips with backwards spans", { backwards <- list( - arr = c("23:58:00", "23:59:00", "00:01:00", "00:30:00", "00:31:00"), - dep = c("23:58:00", "23:59:00", "00:01:00", "00:30:00", "00:31:00") + arr = c("08:58:00", "08:59:00", "08:57:00", "09:26:00", "09:27:00"), + dep = c("08:58:00", "08:59:00", "08:57:00", "09:26:00", "09:27:00") ) forwards <- list( arr = c("08:00:00", "08:10:00", "08:20:00", "08:30:00", "08:40:00"), @@ -381,8 +410,8 @@ test_that("doesn't warn about backwards spans whose length is unknown", { feed <- synthetic_feed( list( list( - arr = c("23:58:00", "23:59:00", "00:01:00", "00:30:00", "00:31:00"), - dep = c("23:58:00", "23:59:00", "00:01:00", "00:30:00", "00:31:00") + arr = c("08:58:00", "08:59:00", "08:57:00", "09:26:00", "09:27:00"), + dep = c("08:58:00", "08:59:00", "08:57:00", "09:26:00", "09:27:00") ) ), route_id = "r1", @@ -608,7 +637,7 @@ test_that("caps only the trips departing within from and to", { }) test_that("doesn't warn about backwards spans outside the segment", { - times <- c("23:58:00", "23:59:00", "00:01:00", "00:30:00", "00:31:00") + times <- c("08:58:00", "08:59:00", "08:57:00", "09:26:00", "09:27:00") feed <- synthetic_feed(list(list(arr = times, dep = times)), route_id = "r1") expect_no_warning(tester(feed, first_stop = "s3")) diff --git a/tests/testthat/test-set_min_dwell_time.R b/tests/testthat/test-set_min_dwell_time.R index 35c4ea48..da467857 100644 --- a/tests/testthat/test-set_min_dwell_time.R +++ b/tests/testthat/test-set_min_dwell_time.R @@ -3,7 +3,8 @@ spo_gtfs <- read_gtfs(spo_path) # creates a feed with three trips whose intermediate stops have dwell times # below and above 30 seconds. trip A runs past midnight, with its later times -# written as "00:01:00" and "00:10:00", and trip C visits stop Y twice +# written as "00:01:00" and "00:10:00" (fixed to "24:01:00" and "24:10:00" by +# the function), and trip C visits stop Y twice dwell_feed <- function() { feed <- list( @@ -92,13 +93,13 @@ test_that("raises only shorter dwell times at intermediate stops", { # dwell times below 30 seconds are raised and the later times shifted. # longer dwell times are kept, and so are the first and last stops of each - # trip. Z's negative dwell time is raised too, so its departure and the - # later times are shifted by about a day and written past 24:00:00 + # trip. A's times after midnight are fixed first, so Z's dwell time is 120 + # seconds and is kept expect_identical( times$arrival_time, c( - "08:00:00", "08:05:00", "08:10:30", "23:59:30", "24:09:00", + "08:00:00", "08:05:00", "08:10:30", "23:59:30", "24:10:30", "09:00:00", "09:05:00", "09:10:30", "10:00:00", "10:05:00", "10:10:30", "10:20:30" ) @@ -106,7 +107,7 @@ test_that("raises only shorter dwell times at intermediate stops", { expect_identical( times$departure_time, c( - "08:00:00", "08:05:30", "08:12:30", "24:00:00", "24:09:00", + "08:00:00", "08:05:30", "08:12:30", "24:01:30", "24:10:30", "09:00:00", "09:05:30", "09:10:30", "10:00:00", "10:05:30", "10:12:30", "10:20:30" ) @@ -117,13 +118,22 @@ test_that("raises only shorter dwell times at intermediate stops", { expect_silent(set_again <- tester(result)) expect_equal(set_again, result) - # a minimum of 0 raises only Z's negative dwell time + # a minimum of 0 raises only negative dwell times. this feed has none once + # A's times are fixed, so only these change - zero_min <- tester(min_dwell_time = 0)$stop_times - expect_identical(zero_min$departure_time[4:5], c("23:59:00", "24:08:00")) + zero_min <- tester(min_dwell_time = 0) + expect_equal(zero_min, fix_overnight_times(dwell_gtfs), ignore_attr = TRUE) + + # B departs from Y a minute before arriving there + + negative_gtfs <- dwell_feed() + negative_gtfs$stop_times[7, departure_time := "09:04:00"] + zero_min <- tester(negative_gtfs, min_dwell_time = 0)$stop_times + expect_identical(zero_min$departure_time[7:8], c("09:05:00", "09:11:00")) + expect_identical(zero_min$arrival_time[8], "09:11:00") expect_identical( - zero_min$departure_time[-(4:5)], - dwell_gtfs$stop_times$departure_time[-(4:5)] + zero_min$departure_time[-(4:8)], + dwell_gtfs$stop_times$departure_time[-(4:8)] ) }) @@ -139,7 +149,11 @@ test_that("never shortens dwell times, whatever the trips and stops given", { # the first stops of the trips are never changed expect_silent(first_stops <- tester(stop_id = "s0")) - expect_equal(first_stops, dwell_gtfs, ignore_attr = TRUE) + expect_equal( + first_stops, + fix_overnight_times(dwell_gtfs), + ignore_attr = TRUE + ) # B's last stop has no departure time, so Y is B's last stop with both times # and keeps its dwell time diff --git a/tests/testthat/test-set_route_frequency.R b/tests/testthat/test-set_route_frequency.R index 46a2d359..7baa67ef 100644 --- a/tests/testthat/test-set_route_frequency.R +++ b/tests/testthat/test-set_route_frequency.R @@ -402,3 +402,28 @@ test_that("route_type restricts the changed routes", { class = "gtfstools_missing_selection_error" ) }) + +test_that("fixes times after midnight written as early morning times", { + # F departs every 30 minutes from 23:00 to "01:00:00". setting it to every + # 10 minutes from 23:30 to 24:30 replaces those departures, without + # doubling them + + gtfs <- overnight_feed() + result <- set_route_frequency( + gtfs, + "R2", + headway = 10, + from = "23:30:00", + to = "24:30:00" + ) + expect_identical(result$frequencies$end_time[3], "25:00:00") + + frequency <- get_route_frequency( + result, + "R2", + from = "23:30:00", + to = "24:30:00" + ) + expect_identical(frequency$departures, 6L) + expect_equal(frequency$mean_headway, 10) +}) diff --git a/tests/testthat/test-set_trip_speed.R b/tests/testthat/test-set_trip_speed.R index b12e59a6..8eae500b 100644 --- a/tests/testthat/test-set_trip_speed.R +++ b/tests/testthat/test-set_trip_speed.R @@ -663,9 +663,13 @@ test_that("segment and time of day args work by reference and on other feeds", { to = "09:00:00" ) ) + # the other trips keep their times, except that times after midnight + # written as early morning times are fixed + + fixed_poa <- fix_overnight_times(poa_gtfs) expect_identical( new_poa$stop_times[!trip_id %chin% in_window], - poa_gtfs$stop_times[!trip_id %chin% in_window] + fixed_poa$stop_times[!trip_id %chin% in_window] ) expect_false(identical( new_poa$stop_times[trip_id %chin% in_window], @@ -760,3 +764,29 @@ test_that("raises errors due to incorrect route_type and selection", { set_trip_speed(no_trips_routeid_gtfs, speed = 50, route_type = 2) ) }) + +test_that("fixes times after midnight written as early morning times", { + poa_gtfs <- read_gtfs( + system.file("extdata/poa_gtfs.zip", package = "gtfstools") + ) + trip <- "T2-1@1#2310" + + # the returned feed has every trip's times after midnight fixed + + expect_no_warning( + result <- set_trip_speed(poa_gtfs, trip, 20), + class = "gtfstools_backwards_times" + ) + expect_equal(get_trip_speed(result, trip)$speed, 20, tolerance = 1e-3) + others <- result$stop_times[trip_id != trip] + fixed_poa <- fix_overnight_times(poa_gtfs) + expect_identical(others, fixed_poa$stop_times[trip_id != trip]) + + # by reference, the given feed gets the fixed times and the new speed + + given_gtfs <- read_gtfs( + system.file("extdata/poa_gtfs.zip", package = "gtfstools") + ) + expect_invisible(set_trip_speed(given_gtfs, trip, 20, by_reference = TRUE)) + expect_equal(given_gtfs$stop_times, result$stop_times, ignore_attr = TRUE) +}) diff --git a/vignettes/editing.Rmd b/vignettes/editing.Rmd index 1141ee8a..fdcdb589 100644 --- a/vignettes/editing.Rmd +++ b/vignettes/editing.Rmd @@ -39,7 +39,7 @@ GTFS feeds describe the transit services of a city or region as they are. Quite - `merge_gtfs()`: combines several feeds into a single one; - `remove_duplicates()`: removes duplicated entries from the feed's tables; - `remove_unused_ids()`: removes the ids that the feed doesn't use, with their entries in every table; -- `fix_gtfs()`: cleans a feed in one call, removing duplicated entries and unused ids, optionally capping the speed of trips, filling blank stop times, optionally setting a minimum dwell time at intermediate stops and recalculating the distances travelled along shapes (replacing any existing ones). +- `fix_gtfs()`: cleans a feed in one call, removing duplicated entries and unused ids, fixing times after midnight written as early morning times, optionally capping the speed of trips, filling blank stop times, optionally setting a minimum dwell time at intermediate stops and recalculating the distances travelled along shapes (replacing any existing ones). This vignette will introduce you to the first four functions and to `fix_gtfs()`, and will cover their usage in detail. By default, they return an edited copy of the feed and leave the given GTFS object unchanged. @@ -179,7 +179,7 @@ interpolated_gtfs$stop_times |> Interpolated stops get equal arrival and departure times (i.e. no dwell time), rounded to the nearest second, and `timepoint = 0` when the `stop_times` table includes a `timepoint` column. In feeds without shapes, use `method = "euclidean"` to measure distances as straight lines between stops instead. -If `trip_id` is not given, the function interpolates the data to give explicit stop times at every stop of every trip in the feed. Some stops may be left blank, with a warning: for example, a few trips of Porto Alegre's feed list their times after midnight as `"00:xx:xx"`, instead of `"24:xx:xx"`, so they seem to arrive before they depart and can't be interpolated. +If `trip_id` is not given, the function interpolates the data to give explicit stop times at every stop of every trip in the feed. Some stops may be left blank, with a warning: for example, stops of trips not linked to a shape, or between times that go back. A few trips of Porto Alegre's feed list their times after midnight as `"00:xx:xx"`, instead of `"24:xx:xx"`, so they seem to arrive before they depart: these times are fixed first, with `fix_overnight_times()`, so these trips are interpolated too. # Fixing common problems @@ -199,10 +199,11 @@ With `fix_gtfs()`, the feed goes through the following steps, in this order: 1. `remove_duplicates()` removes duplicated entries from every table; 2. `remove_unused_ids()` removes the trips without stop times and the entries of every table that no remaining trip uses (e.g. stops that are not used in any trip); -3. `set_max_speed()`, only if `max_speed` is given, slows down the parts of trips that are faster than `max_speed`; -4. `interpolate_stop_times()` fills blank stop times. Since this runs after the speed cap, the filled times follow the capped ones; -5. `set_min_dwell_time()` forces every vehicle to remain stopped at each intermediate stop for at least `min_dwell_time` for passengers to board and alight; -6. `interpolate_shape_dist_traveled()`, in feeds with shapes, adds the distances, in meters, travelled from the start of each shape to each stop and shape point. +3. `fix_overnight_times()` rewrites times after midnight written as early morning times (e.g. `"00:30:00"` instead of `"24:30:00"`), as required by the GTFS specification; +4. `set_max_speed()`, only if `max_speed` is given, slows down the parts of trips that are faster than `max_speed`; +5. `interpolate_stop_times()` fills blank stop times. Since this runs after the speed cap, the filled times follow the capped ones; +6. `set_min_dwell_time()` forces every vehicle to remain stopped at each intermediate stop for at least `min_dwell_time` for passengers to board and alight; +7. `interpolate_shape_dist_traveled()`, in feeds with shapes, adds the distances, in meters, travelled from the start of each shape to each stop and shape point. **Note:** diff --git a/vignettes/graphs.Rmd b/vignettes/graphs.Rmd index 4d827149..b5e42c8b 100644 --- a/vignettes/graphs.Rmd +++ b/vignettes/graphs.Rmd @@ -86,9 +86,6 @@ of these inconsistencies. We also want to ensure that no trips exceed 25 km/h an gtfs <- fix_gtfs(gtfs, max_speed = 25, min_dwell_time = 30) ``` -obs. in this case, `fix_gtfs()` throws two warnings that refer to a few late night trips whose times go back to "00:00:00" after midnight. These trips depart outside the time of day used below, so they don't affect the network. - - # Building a GMNS network By default, `build_graph()` returns the GMNS `node`, `link` and `config` From 25a3f739136cfa7b97daaff1a6f592ac7b5b2e4e Mon Sep 17 00:00:00 2001 From: rafapereirabr Date: Sun, 1 Nov 2026 14:32:50 -0300 Subject: [PATCH 5/6] fix_overnight_times in fix_gtfs --- R/fix_gtfs.R | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/R/fix_gtfs.R b/R/fix_gtfs.R index f799ea5c..6ed7982b 100644 --- a/R/fix_gtfs.R +++ b/R/fix_gtfs.R @@ -142,13 +142,14 @@ fix_gtfs <- function( checkmate::assert_logical(overwrite_stop_times, len = 1, any.missing = FALSE) # remove duplicates first, so the other steps don't see repeated rows - gtfs_fixed <- remove_duplicates(gtfs) # remove unused ids - gtfs_fixed <- remove_unused_ids(gtfs_fixed) + # fix overnight times + gtfs_fixed <- fix_overnight_times(gtfs_fixed) + # cap max speed. this is done before interpolating, so the interpolated # times follow the capped times From fbe9a1690a72deeab64675a7be0d8d2f5dc47e3d Mon Sep 17 00:00:00 2001 From: rafapereirabr Date: Sun, 1 Nov 2026 14:52:55 -0300 Subject: [PATCH 6/6] update news --- DESCRIPTION | 2 +- NEWS.md | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 384801a3..cdda045d 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -65,4 +65,4 @@ NeedsCompilation: yes Roxygen: list(markdown = TRUE) LinkingTo: cpp11 -Config/roxygen2/version: 8.1.0 +Config/roxygen2/version: 8.1.1 diff --git a/NEWS.md b/NEWS.md index f91b9b8d..a34b8a92 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,21 +1,23 @@ # gtfstools (development version) -## Breaking changes +## Bug fixes + +- gtfstools functions now accept `dt_gtfs` objects whose tables were turned into tibbles, grouped data frames or plain data frames by other packages (e.g. after `dplyr::group_by()`). With `by_reference = TRUE`, an object with any such table now raises an informative error, since the edits can't be made by reference on a converted copy: convert it first with `as_dt_gtfs()` (#130, reported by @baarthur, who also opened PR #132). +- Several functions that involved estimating trip speed and duration would +drop trips that cross midnight if GTFS input had times in 00:MM:SS, which +disrespects the GTFS standard to report times as 24:MM:SS in such cases. +This bug has been fixed. Now all functions that calculate or change durations +and speeds now fix these times using a new function `fix_overnight_times()` +and return correct estimates for overnight trips. -- In `set_dwell_time()`, the arguments after `unit` must now be passed by name, since the new `strategy` argument comes right after it. ## New features - New function `fix_overnight_times()` rewrites the times of trips that run past midnight but are written as early morning times (e.g. `"00:30:00"` instead of `"24:30:00"`), as required by the GTFS specification. `fix_gtfs()` now runs it before capping speeds and filling blank stop times, so these trips are no longer left with blank times. -- The functions that calculate or change durations and speeds (`get_trip_duration()`, `get_trip_segment_duration()`, `get_trip_speed()`, `get_route_speed()`, `get_dwell_time()`, `set_trip_speed()`, `set_route_speed()`, `set_max_speed()`, `set_min_speed()`, `set_dwell_time()` and `set_min_dwell_time()`), `interpolate_stop_times()` and `build_graph()` now fix these times the same way before using them, so they give correct results for trips that run past midnight. The functions that return a GTFS object return it with the times fixed. Their `from` and `to` arguments are compared with the fixed times, so a visit written as `"00:30:00"` after midnight is now matched by a time of day such as `"24:00:00"`-`"25:00:00"`, not `"00:00:00"`-`"01:00:00"`, and `build_graph()` places these trips at their real times. -- The functions that use a time of day (`filter_by_time_of_day()`, `get_route_frequency()`, `set_route_frequency()`, `get_stop_frequency()`, `get_vkt()`, `get_vmt()` and `get_trip_length()` with `by = "segment"`) and `get_summary()` also fix these times first, as well as `frequencies` entries whose `end_time` is earlier than their `start_time` (e.g. `"23:00:00"`-`"01:00:00"`). `get_route_frequency()` and the functions based on it used to count a trip that runs past midnight at its time written as an early morning time, instead of at its real departure. `filter_by_time_of_day()` and `set_route_frequency()` return the feed with these times fixed. +- `interpolate_stop_times()` gains `overwrite` and `keep_dwell_times`. With `overwrite = TRUE`, the times of every stop between the first and the last stop of each trip are replaced by interpolated ones, instead of only the blank ones. By default (`keep_dwell_times = TRUE`), these stops keep their original dwell times. - `fix_gtfs()` gains `overwrite_stop_times`. With `overwrite_stop_times = TRUE`, the times of every stop between the first and the last stop of each trip are replaced by interpolated ones, keeping their dwell times, instead of only filling blank times (see `interpolate_stop_times()`). - `set_dwell_time()` gains `strategy`. The default, `"shift_trip_end"`, shifts the later times of the trips, as before. `"keep_total_duration"` splits the change around each intermediate stop, keeping the trip durations. -- `interpolate_stop_times()` gains `overwrite` and `keep_dwell_times`. With `overwrite = TRUE`, the times of every stop between the first and the last stop of each trip are replaced by interpolated ones, instead of only the blank ones. By default (`keep_dwell_times = TRUE`), these stops keep their original dwell times. - -## Bug fixes -- gtfstools functions now accept `dt_gtfs` objects whose tables were turned into tibbles, grouped data frames or plain data frames by other packages (e.g. after `dplyr::group_by()`), converting them back to `data.table` without modifying the input. `as_dt_gtfs()` does the same conversion. With `by_reference = TRUE`, an object with any such table now raises an informative error, since the edits can't be made by reference on a converted copy: convert it first with `as_dt_gtfs()` (#130, reported by @baarthur, who also opened PR #132). # gtfstools 2.0.0