Simplify overnight association handling. - #160
Conversation
f14af69 to
0fbe9f0
Compare
An associated schedule will always be published in its own date, however, a flag (duplicateOvernightAssociations) can be specified to duplicate them so that they run on the same service day of the base schedule.
0fbe9f0 to
f872f5f
Compare
…nd processing late night services Also renamed AddLateNightServices.ts to ShiftLateNightServices.ts to better reflect what it actually does now, and to rename some tests as well.
0431793 to
817b669
Compare
planarnetwork#152 landed --remove-passing-points on the same four files this branch touches. Both options are kept. duplicateOvernightAssociations joins the config's known keys, without which master's parseConfig would refuse it outright, and is read with the boolean() that refuses removePassingPoints: "no".
Follow-up review fixes on top of the three commits below. `needToDuplicate` read the date indicator alone, but shiftLateNightServices moves the base and the associated schedule onto the previous service day independently, so the indicator is not what says how far apart the two end up. A next day association whose base leaves at 00:45 is two days from its portion, and a copy closes one - it was published anyway, leaving the same train in the feed twice and still coupling across a day. The gap is now counted, and a copy made only where it closes it. duplicateOvernightAssociations was read into BuildConfig and dropped: build.ts carries today, range and links into buildContext and did not carry this, so a config asking for it got a feed without it and nothing to say so. It is bridged like the others, added to the config's known keys - master's parseConfig would have refused it outright - and read with the same boolean() that refuses `removePassingPoints: "no"`. GTFS_DUPLICATE_OVERNIGHT_ASSOCIATIONS is read as `=== "1"` the way GTFS_LINKS is; Boolean() took "0" and "false" for a yes. Schedule.clone keeps its id argument required, since resolveLinks names a trip by it and two schedules sharing one couples the wrong pair of trains. The default moves onto copyToPreviousServiceDay, which is the one caller that means to keep it. shiftLateNightServices no longer takes an IdGenerator it stopped using, and the +24h shift is one function rather than two. A BASELINE.md entry for the golden, which CI requires and which the F4 entry now points forward to, and a changeset: the gtfs API breaks and the feed loses a trip id consumers may hold. Tests for the flag off through applyAssociations, for the two gaps a copy cannot close, for the config key reaching the build, and for the ids.
|
Publishing the portion on its own day and putting the copy behind a flag reads much better than moving it and copying back. I've merged master in and pushed a commit with fixes rather than send you round again. Nothing rewritten, so a pull will be clean. The duplication condition read the date indicator on its own, but shiftLateNightServices moves the base and the portion onto the previous service day independently, so the indicator isn't what decides how far apart they land. A next day association whose base leaves at 00:45 sits two days from its portion and a copy closes one, so it published a copy anyway that still coupled across a day. It counts the gap now. Your guard on the portion being late night was right and I kept it, the copy would land exactly where the shift is about to put asDated. duplicateOvernightAssociations never reached the build. parseConfig read it into BuildConfig and nothing carried it across to BuildContext, so a config asking for it got a feed without it and no error to say so. Worth knowing master's parseConfig would have rejected the key outright too, #152 added an allowlist. The environment variable went through Boolean(), which takes "0" and "false" for a yes. Reads like GTFS_LINKS now. |
|
Let me know if it looks good to you and I will merge |
2 participants
This is a follow up to #158. That PR makes associated schedule run on the base service date, and create a copy back on its own. This is wrong because it will result in duplicated services if a departure board is shown from the timetable. Also, the move day then copy back operation is unnecessarily complicated as well.
According to the GTFS standard, it doesn't require transfers to be run on the same service day, however, some legacy journey planners may have difficulty joining trips which run across different service days.
Therefore, I suggest simplify the operation to produce the correct data by default, i.e. running the associated schedule as dated (unless it departs before 02:00 which will be move back to the end of the previous service day), and only duplicate the associated schedule to the base's service day when a flag (
duplicateOvernightAssociations) is specified.The association application now has three schedule members instead of two:
asDated,duplicatedandunassociatedwhich are required for data correctness. Using a hypothetical example of a Brighton - Haywards Heath train joining into the back of an Eastbourne - London Victoria train which departed the previous day (i.e. the Brighton train has a "next day" association),asDatedandduplicatedneeds to return a train heading for "London Victoria", on the original date and on the Eastbourne portion date respectively, whileunassociatedneeds to return a train heading for "Haywards Heath" for any dates where the association does not apply.When
duplicateOvernightAssociationsare on, the linked trips are between schedules which depart on the same service day.The Golden Feed is now generated with the option switched off. The difference to the previous is that the duplicated trips no longer occurs, and the overnight transfers now refer to the next day's schedule.