Add code to expand transfers to/from stations. - #2410
Conversation
landonreed
left a comment
landonreed
left a comment
There was a problem hiding this comment.
@mattwigway, LGTM. Only thing is a nit-picky question about variable modifiers.
| private GtfsStopContext context = new GtfsStopContext(); | ||
|
|
||
| // the location types for transfers.txt | ||
| public int STOP_LOCATION_TYPE = 0; |
There was a problem hiding this comment.
should STOP_LOCATION_TYPE and PARENT_STATION_LOCATION_TYPE be static final?
|
Oh, yep they should, good catch.
…On Mar 2, 2017 1:45 PM, "Landon Reed" ***@***.***> wrote:
***@***.**** commented on this pull request.
@mattwigway <https://github.com/mattwigway>, LGTM. Only thing is a
nit-picky question about variable modifiers.
------------------------------
In src/main/java/org/opentripplanner/routing/edgetype/factory/
GTFSPatternHopFactory.java
<#2410 (comment)>
:
> @@ -306,6 +306,10 @@ public int compare(IndexedLineSegment a, IndexedLineSegment b) {
private GtfsStopContext context = new GtfsStopContext();
+ // the location types for transfers.txt
+ public int STOP_LOCATION_TYPE = 0;
should STOP_LOCATION_TYPE and PARENT_STATION_LOCATION_TYPE be static final
?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2410 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAimrjYb14e9-FIIML46VLyHYqHlzd-Oks5rhw4rgaJpZM4MQMRf>
.
|
abyrd
left a comment
abyrd
left a comment
There was a problem hiding this comment.
Thanks @mattwigway. I don't understand your initial description of the PR though. You say "When a parent station is listed as the to or from station in transfers.txt, we create transfers to/from all child stops... This patch causes it to instead expand parent station transfers into transfers between all child stops of those parent stations."
Isn't the current situation that we don't create transfers between child stops, and we instead attempt to make transfers between the parent stations, which don't exist as vertices?
That's what this PR appears to fix, and it looks good for that purpose. If you can add the "static final" modifiers and update the PR description for posterity, I think this should be merged.
|
Fixed. |
|
Hi, thanks for pursuing this issue!
Obviously, it happens directly after the message "Done finding interlining trips and creating the corresponding edges." occurs... Thank you very much for your support! By the way, do you know a workaround to avoid this issue until it is solved, e.g. by manipulating the GTFS-Files? |
|
This change has not yet made it into master. @abyrd can you confirm that this is ready? |
|
Is this problem fixed for otp-1.1.0-shaded.jar ( from: https://repo1.maven.org/maven2/org/opentripplanner/otp/ ) Because it occured for me today when trying to build a new graph. |
|
@muenchhagen no, I merged it just after the 1.1.0 release, so you'd need to use a newer build to get this fix. All more recent builds should be at https://oss.sonatype.org/content/repositories/staging/org/opentripplanner/otp/1.2.0-SNAPSHOT/ |
|
Today I tried it again, and now it worked fine, the graph was successfully created! Thank you so much for the quick troubleshooting! |
5 participants
When a parent station is listed as the to or from station in transfers.txt, this PR creates transfers
to/from all child stops.
Currently, parent station transfers cause OTP to crash because there are no graph vertices associated with these stations. This patch causes it to instead expand parent station transfers into transfers between all child stops of those parent stations. The only concern here is that it could lead to large transfer tables, but I don't really see a way around that.
Fixes #2404 and possibly #2406.