Enforce flex rule: at least two stop times per trip - #6828
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6828 +/- ##
=============================================
+ Coverage 71.99% 72.08% +0.08%
- Complexity 19437 19521 +84
=============================================
Files 2100 2105 +5
Lines 78774 78972 +198
Branches 7963 8009 +46
=============================================
+ Hits 56717 56927 +210
+ Misses 19249 19221 -28
- Partials 2808 2824 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e9fcd82 to
329cffe
Compare
| // result.add(new ContinuousPickupDropOffTrip(trip, stopTimes)); | ||
| } else if ( | ||
| stopTimes.size() < 2 && | ||
| stopTimes.stream().anyMatch(st -> st.hasFlexWindow() || st.hasFlexibleStop()) |
There was a problem hiding this comment.
What is the need for this check? Can these stoptimes be any stop times or are they filtered somewhere to be just "flex" stop times? Should this check be moved to be the first if so you don't have to account for this edge case in the other checks?
There was a problem hiding this comment.
These are all stop times, not just the flex ones therefore you need something like that.
If I took your comment literally, then I can move the longish if statement to the top and that would look like that: leonardehrenfried@ddf302a
However, I don't find this any better.
Thinking a bit bigger, we could have a first step where I extract only the flex stop times and in a second one we sort it into the two types of flex trips and add the issues.
What do you think of that?
…sMapper.java Co-authored-by: Joel Lappalainen <lappalj8@gmail.com>
vpaturet
left a comment
There was a problem hiding this comment.
Remark: Some flex trips consist in a single area where passengers can be picked up and then dropped off. In practice these trips are modeled with 2 flexible stops that refer to the same area. In this case, the invariant "at least 2 stops" still holds true.
Summary
Both the GTFS and NeTEx specs require flex trips, just like regular ones, to have at least 2 stop times/passing times.
Even though our flex routing engine doesn't produce a result when you have a trip with a single stop time, we nevertheless import the data anyway and then do nothing with it. It may end up in API response which can be quite confusing.
For this reason, this PR rejects flex trips with fewer than 2 stop times and adds an issue to the store.
Issue
No.
Unit tests
Added.