Skip to content

Bugfix for interchanges that start and end from the same stop#4597

Merged
JonathanBrynhagen merged 9 commits into
dev-2.xfrom
interchange_duplicateStops
Dec 6, 2022
Merged

Bugfix for interchanges that start and end from the same stop#4597
JonathanBrynhagen merged 9 commits into
dev-2.xfrom
interchange_duplicateStops

Conversation

@JonathanBrynhagen

Copy link
Copy Markdown
Contributor

Summary

Fixes a bug for interchanges that start end end from the same stop.

Issue

#4572

@JonathanBrynhagen
JonathanBrynhagen requested a review from a team as a code owner November 14, 2022 13:15
@codecov-commenter

codecov-commenter commented Nov 14, 2022

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 20.58824% with 27 lines in your changes missing coverage. Please review.

Project coverage is 60.68%. Comparing base (4a0796c) to head (a0c93b4).
Report is 7574 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
.../opentripplanner/netex/mapping/TransferMapper.java 0.00% 18 Missing ⚠️
...g/opentripplanner/gtfs/mapping/TransferMapper.java 43.75% 3 Missing and 6 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #4597      +/-   ##
=============================================
+ Coverage      60.63%   60.68%   +0.04%     
- Complexity     12229    12245      +16     
=============================================
  Files           1580     1584       +4     
  Lines          63461    63486      +25     
  Branches        6994     6993       -1     
=============================================
+ Hits           38481    38526      +45     
+ Misses         22801    22784      -17     
+ Partials        2179     2176       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hannesj hannesj added !Bug Apply to issues describing a bug and PRs witch fixes it. Entur Test This is currently being tested at Entur labels Nov 14, 2022
@hannesj hannesj added this to the 2.3 milestone Nov 14, 2022
@leonardehrenfried

Copy link
Copy Markdown
Member

Would it be too much to ask for a unit test, at least for the GTFS case? I see that there is a TransferMapperTest where all tests are commented out.

@hannesj
hannesj self-requested a review November 15, 2022 10:07
hannesj
hannesj previously approved these changes Nov 21, 2022
Comment thread src/main/java/org/opentripplanner/gtfs/mapping/TransferMapper.java Outdated
Comment thread src/main/java/org/opentripplanner/netex/mapping/TransferMapper.java Outdated
Comment thread src/main/java/org/opentripplanner/netex/mapping/TransferMapper.java Outdated
Comment thread src/main/java/org/opentripplanner/netex/mapping/TransferMapper.java Outdated
Comment thread src/main/java/org/opentripplanner/netex/mapping/TransferMapper.java Outdated
@MikaelJarfors MikaelJarfors added the Skanetrafiken On skanetrafikens roadmap label Nov 28, 2022
t2gran
t2gran previously approved these changes Dec 4, 2022
Comment thread src/main/java/org/opentripplanner/gtfs/mapping/TransferMapper.java Outdated
…java

Co-authored-by: Thomas Gran <t2gran@gmail.com>
Comment thread src/main/java/org/opentripplanner/gtfs/mapping/TransferMapper.java Outdated
Comment thread src/main/java/org/opentripplanner/gtfs/mapping/TransferMapper.java Outdated
Comment thread src/main/java/org/opentripplanner/gtfs/mapping/TransferMapper.java Outdated
Jonathan Brynhagen added 2 commits December 5, 2022 17:15
Comment thread src/main/java/org/opentripplanner/gtfs/mapping/TransferMapper.java Outdated
hannesj
hannesj previously approved these changes Dec 5, 2022
@JonathanBrynhagen
JonathanBrynhagen merged commit 3d063bc into dev-2.x Dec 6, 2022
@JonathanBrynhagen
JonathanBrynhagen deleted the interchange_duplicateStops branch December 6, 2022 09:18
t2gran pushed a commit that referenced this pull request Dec 6, 2022
vpaturet added a commit to entur/OpenTripPlanner that referenced this pull request Mar 25, 2026
When a ServiceJourneyInterchange references a ScheduledStopPoint that
appears multiple times in the feeder trip's JourneyPattern (loop route),
TransferMapper previously used lastIndexOf to pick a single stop
position for the FROM side. This was introduced in opentripplanner#4597 to handle
circular routes where the stop is both first and last. However, for
loop-in-middle patterns (stop visited at positions 30 and 44 in a
45-stop pattern), lastIndexOf picks the wrong occurrence.

Since we cannot know at import time which visit Raptor will use for the
transfer, TransferMapper now creates a ConstrainedTransfer for each
occurrence of the stop. This handles both circular routes and
loop-in-middle patterns correctly.

Addresses opentripplanner#7466
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

!Bug Apply to issues describing a bug and PRs witch fixes it. Entur Test This is currently being tested at Entur Skanetrafiken On skanetrafikens roadmap

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Problem with interchanges that start and stop on the same StopPoint

6 participants