Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use last predicted stop as added trip headsign #839

Merged
merged 1 commit into from
Nov 19, 2024
Merged

feat: use last predicted stop as added trip headsign #839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 12 additions & 36 deletions apps/state/lib/state/trip/added.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ defmodule State.Trip.Added do
with %{route_pattern_id: route_pattern_id} when is_binary(route_pattern_id) <- prediction,
%{representative_trip_id: rep_trip_id} <- State.RoutePattern.by_id(route_pattern_id),
[trip | _] <- State.Trip.by_id(rep_trip_id) do
stop = parent_or_stop(prediction.stop_id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would this make the headsign different for each prediction? it seems like it's applying it to every prediction along the trip rather than applying the last stop's prediction's stopname as the headsign

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I should've called this out. Only the last prediction ever makes it to this function due to the reduce here. I can add the requested test to help demonstrate that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah that makes sense, misread it as a simple sorting rather than just keeping the last 👍


headsign = if stop, do: stop.name, else: trip.headsign

[
%{
trip
Expand All @@ -64,7 +68,8 @@ defmodule State.Trip.Added do
service_id: nil,
wheelchair_accessible: 1,
bikes_allowed: 0,
revenue: prediction.revenue
revenue: prediction.revenue,
headsign: headsign
}
]
else
Expand All @@ -74,25 +79,7 @@ defmodule State.Trip.Added do
end

defp prediction_to_trip_via_shape(prediction) do
stop =
case State.Stop.by_id(prediction.stop_id) do
%{parent_station: nil} = stop -> stop
%{parent_station: id} -> State.Stop.by_id(id)
_other -> nil
end

last_stop_id =
[prediction.route_id]
|> State.Shape.select_routes(prediction.direction_id)
|> Stream.filter(&(&1.route_id == prediction.route_id))
|> Enum.find_value(&last_stop_id_on_shape(&1, prediction, stop))

stop =
if is_nil(last_stop_id) or last_stop_id == stop.id do
stop
else
State.Stop.by_id(last_stop_id)
end
stop = parent_or_stop(prediction.stop_id)

if stop == nil do
[]
Expand All @@ -116,22 +103,11 @@ defmodule State.Trip.Added do
end
end

defp last_stop_id_on_shape(_, _, nil), do: nil

defp last_stop_id_on_shape(%{priority: p} = shape, prediction, stop) when p >= 0 do
shape_stops =
State.StopsOnRoute.by_route_id(
prediction.route_id,
direction_id: prediction.direction_id,
shape_ids: [shape.id]
)

if Enum.any?(shape_stops, &(&1 in [stop.id, stop.parent_station])) do
List.last(shape_stops)
defp parent_or_stop(stop_id) do
case State.Stop.by_id(stop_id) do
%{parent_station: nil} = stop -> stop
%{parent_station: id} -> State.Stop.by_id(id)
_other -> nil
end
end

defp last_stop_id_on_shape(_, _, _) do
nil
end
end
23 changes: 14 additions & 9 deletions apps/state/test/state/trip/added_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,15 @@ defmodule State.Trip.AddedTest do
{:ok, %{shape: shape, prediction: prediction}}
end

test "if there's a matching shape for the route/direction, uses the last stop from that shape",
test "if there's a matching shape for the route/direction, uses the last stop predicted stop",
%{prediction: prediction} do
insert_predictions([prediction])
assert [%{headsign: "Last Stop on Shape"}] = by_id(@trip_id)
predictions = [
%{@prediction | stop_sequence: 3, stop_id: "child"},
%{@prediction | stop_sequence: 2, stop_id: "other"}
]

insert_predictions(predictions)
assert [%{headsign: "Parent"}] = by_id(@trip_id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to add multiple predictions to verify that the headsign is being properly set for multiple predictions on the same trip?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test updated to use multiple predictions. There's also this test on for the route_pattern side that tests the same thing so this has us covering both the shape and route pattern sides of this functionality.

end

test "does not use a shape on the wrong route or direction", %{
Expand Down Expand Up @@ -231,13 +236,13 @@ defmodule State.Trip.AddedTest do
test "creates a trip with revenue value set to :REVENUE",
%{prediction: prediction} do
insert_predictions([%{prediction | revenue: :REVENUE}])
assert [%{headsign: "Last Stop on Shape", revenue: :REVENUE}] = by_id(@trip_id)
assert [%{headsign: "Parent", revenue: :REVENUE}] = by_id(@trip_id)
end

test "creates a trip with revenue value set to false",
%{prediction: prediction} do
insert_predictions([%{prediction | revenue: :NON_REVENUE}])
assert [%{headsign: "Last Stop on Shape", revenue: :NON_REVENUE}] = by_id(@trip_id)
assert [%{headsign: "Parent", revenue: :NON_REVENUE}] = by_id(@trip_id)
end
end

Expand Down Expand Up @@ -267,19 +272,19 @@ defmodule State.Trip.AddedTest do
end)
end

test "if there's a matching route_pattern, use the representative trip" do
test "if there's a matching route_pattern, use the last predicted stop" do
insert_predictions([%{@prediction | stop_id: "child"}])
assert [%{headsign: "Headsign"}] = by_id(@trip_id)
assert [%{headsign: "Parent"}] = by_id(@trip_id)
end

test "creates a trip with revenue value set to :REVENUE" do
insert_predictions([%{@prediction | stop_id: "child", revenue: :REVENUE}])
assert [%{headsign: "Headsign", revenue: :REVENUE}] = by_id(@trip_id)
assert [%{headsign: "Parent", revenue: :REVENUE}] = by_id(@trip_id)
end

test "creates a trip with revenue value set to :NON_REVENUE" do
insert_predictions([%{@prediction | stop_id: "child", revenue: :NON_REVENUE}])
assert [%{headsign: "Headsign", revenue: :NON_REVENUE}] = by_id(@trip_id)
assert [%{headsign: "Parent", revenue: :NON_REVENUE}] = by_id(@trip_id)
end
end
end
Loading