fix(SF2.0/UpcomingDepartures): Get scheduled and predicted other_stops #2854
Conversation
joshlarson
force-pushed
the
jdl/sf2.0/ud/cr-fancy-labels
branch
from
a55dfbf to
8b0489a
Compare
joshlarson
force-pushed
the
jdl/sf2.0/ud/refactor-predictions-schedules-fetching
branch
from
3703a0d to
247158b
Compare
joshlarson
force-pushed
the
jdl/sf2.0/ud/refactor-predictions-schedules-fetching
branch
from
247158b to
945f281
Compare
|
|
||
| defp reject_timeless_predictions(predictions) do | ||
| predictions | ||
| defp end_of_trip?(%PredictedSchedule{schedule: schedule, prediction: prediction}) do |
There was a problem hiding this comment.
suggestion: it's unclear from the code alone how "end of trip" means "no predicted or scheduled departure time" -- I suggest either adjusting the name of this function or adding an explanatory comment.
There was a problem hiding this comment.
And there are a bunch of functions in this module at this point that could use explanatory comments, since the whole thing is becoming fairly complicated. I'll probably add more of those over the next few weeks as the behavior in this module settles down (those comments are very helpful, and also quite annoying to refactor around).
| defp trip_details(predictions_by_trip_id, trip_id, stop_id) do | ||
| other_stops = other_stops(predictions_by_trip_id |> Map.get(trip_id)) | ||
| defp trip_details(predicted_schedules_by_trip_id, trip_id, stop_id) do | ||
| other_stops = other_stops(predicted_schedules_by_trip_id |> Map.get(trip_id)) |
There was a problem hiding this comment.
nit/question: This variable is technically all stops, I think?
There was a problem hiding this comment.
Oh, hum, yes.
I'm working on refactoring pretty much everything about trip details in an upcoming PR, so would it be okay to keep this for now, with the understanding that this is almost certainly going to get torn apart and redone pretty soon?
joshlarson
deleted the
jdl/sf2.0/ud/refactor-predictions-schedules-fetching
branch
This fixes an issue present in Ye Olde Schedulle Finderre, as well as previous instances of this, where commuter rail trips would only be endowed with predictions for their first stop until they actually pulled out of the station, like 👇.
Now, instead of checking only schedules or only predictions, we check
predicted_scheduleswhich already has the schedules and predictions stitched together, in order to fallback to schedules when predictions are unavailable, even within a single trip:(Left: SF1.0 without the fix; Right: SF2.0 with the fix)
Asana Ticket: [SF/UD] CR: Use schedule data to fill in trip details when predictions are for a partial trip