Skip to content

Calculate fares from itineraries not Raptor paths, calculate flex fares - #3743

Merged
leonardehrenfried merged 30 commits into
opentripplanner:dev-2.xfrom
ibi-group:flex-fares
Nov 22, 2021
Merged

Calculate fares from itineraries not Raptor paths, calculate flex fares#3743
leonardehrenfried merged 30 commits into
opentripplanner:dev-2.xfrom
ibi-group:flex-fares

Conversation

@leonardehrenfried

Copy link
Copy Markdown
Member

Summary

As discussed in a previous dev meeting, I converted the fare calculation code so it uses Itinerary and not RaptorPath as the input. This makes it easy to calculate the fares for other types of trips that don't have a RaptorPath, like flex trips do.

I tried to keep the old fare calculation code as untouched as possible apart from a single small-ish refactoring, namely converting the times in Ride from untyped longs to ZonedDateTime.

The fares are actually calculated in TransitRouter and FlexRouter but we said that we want to move this to a later phase of the computation, perhaps the filter chain.

Right now this PR depends on OneBusAway/onebusaway-gtfs-modules#183 before it can be merged.

If it isn't merged and released by upstream soon-ish, would you be ok if I published this to Maven Central under my own namespace?

Issue

Fixes #3656

Unit tests

I managed to reactivate very old tests for the fare calculation feature. I also added ones for calculating fares for direct flex searches.

Code style

Yes.

Documentation

Yes.

@leonardehrenfried
leonardehrenfried marked this pull request as ready for review November 12, 2021 14:19
@leonardehrenfried
leonardehrenfried requested a review from a team as a code owner November 12, 2021 14:19
@leonardehrenfried

Copy link
Copy Markdown
Member Author

Review comments:

  • Using a temporary fork of OBA GTFS is ok.
  • Move logic for intermediate stops into mapper.

@t2gran
t2gran self-requested a review November 16, 2021 10:59
@t2gran t2gran added this to the 2.1 milestone Nov 16, 2021
@t2gran
t2gran requested a review from hannesj November 16, 2021 11:00

@t2gran t2gran left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, not much to pick on here, just a big block inside a stream chain.

Comment thread src/ext-test/java/org/opentripplanner/ext/flex/ScheduledDeviatedTripTest.java Outdated
Comment thread src/main/java/org/opentripplanner/routing/fares/impl/NycFareServiceImpl.java Outdated
…edTripTest.java

Co-authored-by: Thomas Gran <t2gran@gmail.com>
@t2gran

t2gran commented Nov 16, 2021

Copy link
Copy Markdown
Member

This PR switches from the "official" onebusaway-gtfs to @leonardehrenfried´s version of the lib. We discussed this at todays dev-meeting and this is ok, while we wait for an approval of the PR OneBusAway/onebusaway-gtfs-modules#183.

@leonardehrenfried
leonardehrenfried force-pushed the flex-fares branch 4 times, most recently from 246b1a8 to 3955701 Compare November 16, 2021 16:53
@leonardehrenfried

leonardehrenfried commented Nov 16, 2021

Copy link
Copy Markdown
Member Author

I moved the check of whether to add the intermediate stops should be added to the REST API layer.

This means that the SnapshotTest had to be modified as well to because the internal model now always has the intermediate stops.

@leonardehrenfried

Copy link
Copy Markdown
Member Author

My PR to the OBA repo has been merged. This means it's no longer necessary to use my forked version.

@hannesj hannesj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎉

import org.opentripplanner.model.StopLocation;

/**
* A set of edges on a single route, with associated information. Used only in calculating fares.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the future, I'd like to get rid of this class, all operations where this is used, should be done on regular Leg objects

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I agree

@leonardehrenfried
leonardehrenfried merged commit a53f6d8 into opentripplanner:dev-2.x Nov 22, 2021
t2gran pushed a commit that referenced this pull request Nov 22, 2021
@leonardehrenfried leonardehrenfried added IBI Developed by or important for IBI Group and removed IBI test labels Jun 21, 2022
@leonardehrenfried
leonardehrenfried deleted the flex-fares branch October 8, 2022 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

IBI Developed by or important for IBI Group

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flex routes don't contain fare information

3 participants