fix: fix logic to compute service window #2029
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| package org.mobilitydata.gtfsvalidator.reportsummary.model; | ||
|
|
||
| import static java.util.stream.Collectors.*; | ||
|
|
||
| import java.time.LocalDate; | ||
| import java.util.*; | ||
| import org.mobilitydata.gtfsvalidator.table.GtfsCalendar; | ||
| import org.mobilitydata.gtfsvalidator.table.GtfsCalendarDate; | ||
| import org.mobilitydata.gtfsvalidator.table.GtfsCalendarDateExceptionType; | ||
| import org.mobilitydata.gtfsvalidator.table.GtfsTrip; | ||
| import org.mobilitydata.gtfsvalidator.util.FuncUtil; | ||
|
|
||
| record ServiceWindow(LocalDate startDate, LocalDate endDate) { | ||
| static Optional<ServiceWindow> fromCalendars( | ||
| List<GtfsTrip> trips, List<GtfsCalendar> allCalendars) { | ||
| Set<String> serviceIds = new HashSet<>(trips.stream().map(GtfsTrip::serviceId).toList()); | ||
sylvansson marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| List<GtfsCalendar> calendars = | ||
| allCalendars.stream() | ||
| .filter(calendar -> serviceIds.contains(calendar.serviceId())) | ||
| .toList(); | ||
|
|
||
| // Only empty if there are no calendars. | ||
| Optional<LocalDate> startDate = | ||
| calendars.stream().map(c -> c.startDate().getLocalDate()).min(LocalDate::compareTo); | ||
| Optional<LocalDate> endDate = | ||
| calendars.stream().map(c -> c.endDate().getLocalDate()).max(LocalDate::compareTo); | ||
| return startDate.map(d -> new ServiceWindow(d, endDate.get())); | ||
| } | ||
|
|
||
| static Optional<ServiceWindow> fromCalendarDates( | ||
| List<GtfsTrip> trips, List<GtfsCalendarDate> allCalendarDates) { | ||
| Set<String> serviceIds = new HashSet<>(trips.stream().map(GtfsTrip::serviceId).toList()); | ||
|
|
||
| List<LocalDate> calendarDates = | ||
| allCalendarDates.stream() | ||
| .filter( | ||
| d -> | ||
| serviceIds.contains(d.serviceId()) | ||
| && d.exceptionType() == GtfsCalendarDateExceptionType.SERVICE_ADDED) | ||
| .map(d -> d.date().getLocalDate()) | ||
| .toList(); | ||
|
|
||
| // Only empty if there are no calendar dates. | ||
| Optional<LocalDate> startDate = calendarDates.stream().min(LocalDate::compareTo); | ||
| Optional<LocalDate> endDate = calendarDates.stream().max(LocalDate::compareTo); | ||
| return startDate.map(d -> new ServiceWindow(d, endDate.get())); | ||
| } | ||
|
|
||
| static Optional<ServiceWindow> fromCalendarsAndCalendarDates( | ||
| List<GtfsTrip> trips, List<GtfsCalendar> calendars, List<GtfsCalendarDate> calendarDates) { | ||
| Optional<ServiceWindow> serviceWindowFromCalendars = | ||
| ServiceWindow.fromCalendars(trips, calendars); | ||
| if (serviceWindowFromCalendars.isEmpty()) { | ||
| return Optional.empty(); | ||
| } | ||
|
|
||
| Set<String> serviceIds = new HashSet<>(trips.stream().map(GtfsTrip::serviceId).toList()); | ||
|
|
||
| Map<String, Set<LocalDate>> removedDaysByServiceId = | ||
|
Contributor
There was a problem hiding this comment. @sylvansson @davidgamez one question, the function seems to create removedDaysByServiceId by using only GtfsCalendarDate (calendar_dates.txt). However, there might be services in So I think the way to check this is by making sure that for each removed date, it is removed for every service_id that figures either in Eg: calendar_dates.txt: The service window is 2-5-2025 to 31-5-2025 because:
I suggest that for each date, we count the total unique removed service_ids, then compare it with the count of unique service_ids that operate on that date (both from
Member
There was a problem hiding this comment. Thanks, @skalexch. You are right. The spec doesn't force all service IDs to be defined in the calendar.txt and calendar_dates.txt when both files are defined. In this case, we can have service IDs from calendar.txt that are not present in the calendar_dates and will have a service on a day removed by a calendar_date entry. Following this logic, I agree that we need to keep a total of "all unique service IDs" per day, so we make sure that all services are not present to mark the date as "removed." The "all unique service IDs" can be computed by looking at the trips.txt file.
Contributor
Author
There was a problem hiding this comment. Good catch, I'll try to get this fixed sometime next week.
Member
There was a problem hiding this comment.
Thanks! |
||
| calendarDates.stream() | ||
| .filter( | ||
| d -> | ||
| d.exceptionType() == GtfsCalendarDateExceptionType.SERVICE_REMOVED | ||
| && serviceIds.contains(d.serviceId())) | ||
| .collect( | ||
| groupingBy( | ||
| GtfsCalendarDate::serviceId, mapping(d -> d.date().getLocalDate(), toSet()))); | ||
|
|
||
| // We compute the set of days that are removed across all services in | ||
| // order to shift the start and end dates. | ||
| Set<LocalDate> removedDays = FuncUtil.intersectAll(removedDaysByServiceId.values()); | ||
|
|
||
| LocalDate startDate = serviceWindowFromCalendars.get().startDate(); | ||
| LocalDate endDate = serviceWindowFromCalendars.get().endDate(); | ||
|
|
||
| while (removedDays.contains(startDate)) { | ||
| startDate = startDate.plusDays(1); | ||
| } | ||
| while (removedDays.contains(endDate)) { | ||
| endDate = endDate.minusDays(1); | ||
| } | ||
| return Optional.of(new ServiceWindow(startDate, endDate)); | ||
| } | ||
|
|
||
| static Optional<ServiceWindow> get( | ||
| List<GtfsTrip> trips, | ||
| Optional<List<GtfsCalendar>> calendars, | ||
| Optional<List<GtfsCalendarDate>> calendarDates) { | ||
| if (calendarDates.isEmpty() && calendars.isPresent()) { | ||
| return ServiceWindow.fromCalendars(trips, calendars.get()); | ||
| } | ||
|
|
||
| if (calendarDates.isPresent() && calendars.isEmpty()) { | ||
| return ServiceWindow.fromCalendarDates(trips, calendarDates.get()); | ||
| } | ||
|
|
||
| if (calendars.isPresent() && calendarDates.isPresent()) { | ||
| return ServiceWindow.fromCalendarsAndCalendarDates( | ||
| trips, calendars.get(), calendarDates.get()); | ||
| } | ||
|
|
||
| return Optional.empty(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package org.mobilitydata.gtfsvalidator.util; | ||
|
|
||
| import static java.util.Collections.emptySet; | ||
|
|
||
| import java.util.Collection; | ||
| import java.util.HashSet; | ||
| import java.util.Set; | ||
|
|
||
| public class FuncUtil { | ||
sylvansson marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| public static <T> Set<T> intersectAll(Collection<Set<T>> sets) { | ||
sylvansson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (sets.isEmpty()) { | ||
| return emptySet(); | ||
| } | ||
|
|
||
| Set<T> intersection = null; | ||
| for (Set<T> set : sets) { | ||
| if (intersection == null) { | ||
| intersection = new HashSet<>(set); | ||
| } else { | ||
| intersection.retainAll(set); | ||
| } | ||
| } | ||
| return intersection; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package org.mobilitydata.gtfsvalidator.util; | ||
|
|
||
| import static com.google.common.truth.Truth.assertThat; | ||
| import static java.util.Collections.emptyList; | ||
| import static java.util.Collections.emptySet; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Set; | ||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
| import org.junit.runners.JUnit4; | ||
|
|
||
| @RunWith(JUnit4.class) | ||
| public class FuncUtilTest { | ||
| @Test | ||
| public void testIntersectAll() { | ||
| assertThat( | ||
| FuncUtil.intersectAll( | ||
| List.of( | ||
| Set.of(1, 2, 3, 4, 5, 6), | ||
| Set.of(1, 2, 3, 4, 5, 6, 7, 8, 9, 10), | ||
| Set.of(2, 4, 6, 8, 10)))) | ||
| .isEqualTo(Set.of(2, 4, 6)); | ||
|
|
||
| assertThat(FuncUtil.intersectAll(emptyList())).isEqualTo(emptySet()); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.