Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add StopTimeTripWithoutTimes validator #1474

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: add StopTimeTripWithoutTimes validator #1474

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd86d42
feat: add StopTimeTripWithoutTimes validator
KClough Feb 24, 2023
9507ed6
Merge branch 'master' into feat/validate-stop-time-trip-without-times
KClough Jun 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions RULES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ Each Notice is associated with a severity: `INFO`, `WARNING`, `ERROR`.
| [`start_and_end_range_out_of_order`](#start_and_end_range_out_of_order) | Two date or time fields are out of order. |
| [`station_with_parent_station`](#station_with_parent_station) | A station has `parent_station` field set. |
| [`stop_time_timepoint_without_times`](#stop_time_timepoint_without_times) | `arrival_time` or `departure_time` not specified for timepoint. |
| [\`stop_time_trip_without_times_notice](#stop_time_trip_without_times) \| `arrival_time` or `departure_time` not specified for stop time trip. | |

| [`stop_time_with_arrival_before_previous_departure_time`](#stop_time_with_arrival_before_previous_departure_time) | Backwards time travel between stops in `stop_times.txt` |
| [`stop_time_with_only_arrival_or_departure_time`](#stop_time_with_only_arrival_or_departure_time) | Missing `stop_times.arrival_time` or `stop_times.departure_time`. |
| [`stop_without_location`](#stop_without_location) | `stop_lat` and/or `stop_lon` is missing for stop with `location_type` equal to`0`, `1`, or `2` |
Expand Down Expand Up @@ -196,6 +198,32 @@ Trips with the same block id have overlapping stop times.

</details>

<a name="StopTimeTripWithoutTimesNotice"/>

### stop_time_trip_without_times

We can't generate the ERROR notice block_trips_with_overlapping_stop_times for the same trip because we are missing time information.

#### References
* [stops.txt specification](http://gtfs.org/reference/static#stopstxt)
* [trips.txt specification](http://gtfs.org/reference/static#tripstxt)

<details>

#### Notice fields description

| Field name | Description | Type |
| ---------------- | --------------------------------------------------- | ------- |
| `csvRowNumber` | The row number from `stops.txt` of the faulty stop. | Long |
| `tripId` | The id of faulty trip. | String |
| `stopSequence` | The faulty record's `stop_times.stop_sequence`. | Integer |
| `specifiedField` | Name of the missing field. | String |

#### Affected files
* [`stops.txt`](http://gtfs.org/reference/static#stopstxt)

</details>

<a name="CsvParsingFailedNotice"/>

### csv_parsing_failed
Expand Down
47 changes: 44 additions & 3 deletions ...org/mobilitydata/gtfsvalidator/validator/BlockTripsWithOverlappingStopTimesValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@
* <p>Both trips have to be operating on the same service day, as determined by comparing the
* service dates of the trips.
*
* <p>Generated notices: {@link BlockTripsWithOverlappingStopTimesNotice}.
* <p>Generated notices: {@link BlockTripsWithOverlappingStopTimesNotice}, {@link
* StopTimeTripWithoutTimesNotice}.
*/
@GtfsValidator
public class BlockTripsWithOverlappingStopTimesValidator extends FileValidator {
Expand Down Expand Up @@ -87,7 +88,8 @@ public void validate(NoticeContainer noticeContainer) {
// properly judge trip overlap.
for (GtfsTripOverlap overlap :
findOverlapIntervals(
constructOrderedTripIntervals(tripsInBlock), serviceIdIntersectionCache)) {
constructOrderedTripIntervals(tripsInBlock, noticeContainer),
serviceIdIntersectionCache)) {
final GtfsTrip tripA = overlap.getTripA();
final GtfsTrip tripB = overlap.getTripB();
noticeContainer.addValidationNotice(
Expand All @@ -106,7 +108,8 @@ public void validate(NoticeContainer noticeContainer) {
*
* <p>Intervals are sorted by increasing first-arrival times, and then last-departure time.
*/
private List<GtfsTripInterval> constructOrderedTripIntervals(List<GtfsTrip> tripsInBlock) {
private List<GtfsTripInterval> constructOrderedTripIntervals(
List<GtfsTrip> tripsInBlock, NoticeContainer noticeContainer) {
ArrayList<GtfsTripInterval> intervals = new ArrayList<>();
intervals.ensureCapacity(tripsInBlock.size());
for (GtfsTrip trip : tripsInBlock) {
Expand All @@ -117,10 +120,22 @@ private List<GtfsTripInterval> constructOrderedTripIntervals(List<GtfsTrip> trip
}
GtfsStopTime firstStopTime = stopTimes.get(0);
GtfsStopTime lastStopTime = stopTimes.get(stopTimes.size() - 1);

if (!firstStopTime.hasArrivalTime()
|| !firstStopTime.hasDepartureTime()
|| !lastStopTime.hasArrivalTime()
|| !lastStopTime.hasDepartureTime()) {

for (var stop : new GtfsStopTime[] {firstStopTime, lastStopTime}) {
if (!stop.hasArrivalTime()) {
noticeContainer.addValidationNotice(
new StopTimeTripWithoutTimesNotice(stop, GtfsStopTime.ARRIVAL_TIME_FIELD_NAME));
}
if (!stop.hasDepartureTime()) {
noticeContainer.addValidationNotice(
new StopTimeTripWithoutTimesNotice(stop, GtfsStopTime.DEPARTURE_TIME_FIELD_NAME));
}
}
continue;
}
intervals.add(
Expand Down Expand Up @@ -306,4 +321,30 @@ static class BlockTripsWithOverlappingStopTimesNotice extends ValidationNotice {
this.intersection = intersection;
}
}

/**
* We can't generate the ERROR notice block_trips_with_overlapping_stop_times for the same trip
* because we are missing time information
Comment on lines +326 to +327
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that stop times are missing and there is no error?

@isabelle-dr can you review this wording?

*
* <p>Severity: {@code SeverityLevel.ERROR}
*/
@GtfsValidationNotice(severity = ERROR)
static class StopTimeTripWithoutTimesNotice extends ValidationNotice {
/** The row number from `stops.txt` of the faulty stop. */
private final int csvRowNumber;
/** The id of faulty trip. */
private final String tripId;
/** The faulty record's `stop_times.stop_sequence`. */
private final long stopSequence;
/** Name of the missing field. */
private final String specifiedField;

StopTimeTripWithoutTimesNotice(GtfsStopTime stopTime, String specifiedField) {
super(ERROR);
this.csvRowNumber = stopTime.csvRowNumber();
this.tripId = stopTime.tripId();
this.stopSequence = stopTime.stopSequence();
this.specifiedField = specifiedField;
}
}
}
74 changes: 74 additions & 0 deletions ...mobilitydata/gtfsvalidator/validator/BlockTripsWithOverlappingStopTimesValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.mobilitydata.gtfsvalidator.type.GtfsTime;
import org.mobilitydata.gtfsvalidator.util.CalendarUtilTest;
import org.mobilitydata.gtfsvalidator.validator.BlockTripsWithOverlappingStopTimesValidator.BlockTripsWithOverlappingStopTimesNotice;
import org.mobilitydata.gtfsvalidator.validator.BlockTripsWithOverlappingStopTimesValidator.StopTimeTripWithoutTimesNotice;

@RunWith(JUnit4.class)
public class BlockTripsWithOverlappingStopTimesValidatorTest {
Expand Down Expand Up @@ -110,6 +111,23 @@ private static List<GtfsStopTime> createStopTimeTable(
return stopTimes;
}

private static GtfsStopTime createStopTime(
int csvRowNumber,
String tripId,
GtfsTime arrivalTime,
GtfsTime departureTime,
String stopId,
int stopSequence) {
return new GtfsStopTime.Builder()
.setCsvRowNumber(csvRowNumber)
.setTripId(tripId)
.setArrivalTime(arrivalTime)
.setDepartureTime(departureTime)
.setStopSequence(stopSequence)
.setStopId(stopId)
.build();
}

private static List<ValidationNotice> generateNotices(
List<GtfsCalendar> calendars,
List<GtfsCalendarDate> calendarDates,
Expand Down Expand Up @@ -209,4 +227,60 @@ public void tripsWith0or1Stop() {
new String[][] {new String[] {"08:00:00"}})))
.isEmpty();
}

@Test
public void stopTimeMissingArrivalTime() {
List<GtfsTrip> trips = createTripTable(new String[] {"t0"}, new String[] {"WEEK"}, "b1");

List<GtfsStopTime> stopTimes = new ArrayList<>();
stopTimes.add(
createStopTime(
1,
"t0",
null, // arrival time
GtfsTime.fromSecondsSinceMidnight(518), // departure time
"s0",
1));
stopTimes.add(
createStopTime(
2,
"t0",
GtfsTime.fromSecondsSinceMidnight(1418), // arrival time
GtfsTime.fromSecondsSinceMidnight(1518), // departure time
"s1",
2));

assertThat(generateNotices(createCalendarTable(), ImmutableList.of(), trips, stopTimes))
.containsExactly(
new StopTimeTripWithoutTimesNotice(
stopTimes.get(0), GtfsStopTime.ARRIVAL_TIME_FIELD_NAME));
}

@Test
public void stopTimeMissingDepartureTime() {
List<GtfsTrip> trips = createTripTable(new String[] {"t0"}, new String[] {"WEEK"}, "b1");

List<GtfsStopTime> stopTimes = new ArrayList<>();
stopTimes.add(
createStopTime(
1,
"t0",
GtfsTime.fromSecondsSinceMidnight(518), // arrival time
null, // Departure Time
"s0",
1));
stopTimes.add(
createStopTime(
2,
"t0",
GtfsTime.fromSecondsSinceMidnight(1418), // arrival time
GtfsTime.fromSecondsSinceMidnight(1518), // departure time
"s1",
2));

assertThat(generateNotices(createCalendarTable(), ImmutableList.of(), trips, stopTimes))
.containsExactly(
new StopTimeTripWithoutTimesNotice(
stopTimes.get(0), GtfsStopTime.DEPARTURE_TIME_FIELD_NAME));
}
}