Skip to content

Expose stop transfer priority in Transmodel API - #5942

Merged
vpaturet merged 2 commits into
opentripplanner:dev-2.xfrom
entur:expose_stop_transfer_priority_in_api
Jul 8, 2024
Merged

Expose stop transfer priority in Transmodel API#5942
vpaturet merged 2 commits into
opentripplanner:dev-2.xfrom
entur:expose_stop_transfer_priority_in_api

Conversation

@vpaturet

@vpaturet vpaturet commented Jul 2, 2024

Copy link
Copy Markdown
Contributor

Summary

This PR exposes stop transfer priority in the TransModel API.
"Stop transfer priority" is used to specify the relative preference of a stop when several stops can be used for a transfer.
In the TransModel API transfers are referred to as interchanges.

The naming of the existing field weighting was confusing and the data fetching was never implemented (it returned always the same value)
It is replaced by stopInterchangePriority whose name matches more clearly the name interchangePriority, that is the priority set on interchanges.
The NeTEx enum values are also confusing, in particular interchangeNotAllowed. It is replaced by discouraged which matches the actual behaviour of OTP (not banning but discouraging the transfer at a given stop).

Issue

No

Unit tests

No

Documentation

Updated API documentation

@codecov

codecov Bot commented Jul 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 80.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 69.45%. Comparing base (4b50da8) to head (a4ab59a).
Report is 13 commits behind head on dev-2.x.

Files Patch % Lines
...transmodel/model/stop/MonoOrMultiModalStation.java 0.00% 3 Missing ⚠️
...nner/apis/transmodel/model/stop/StopPlaceType.java 88.88% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #5942      +/-   ##
=============================================
- Coverage      69.46%   69.45%   -0.01%     
+ Complexity     17076    17069       -7     
=============================================
  Files           1937     1937              
  Lines          73680    73712      +32     
  Branches        7539     7540       +1     
=============================================
+ Hits           51182    51200      +18     
- Misses         19874    19884      +10     
- Partials        2624     2628       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vpaturet
vpaturet force-pushed the expose_stop_transfer_priority_in_api branch from 14cc0a3 to 2a4a8da Compare July 3, 2024 08:03
@vpaturet
vpaturet force-pushed the expose_stop_transfer_priority_in_api branch from 2a4a8da to 10f7464 Compare July 3, 2024 08:41
@vpaturet
vpaturet marked this pull request as ready for review July 3, 2024 09:00
@vpaturet
vpaturet requested a review from a team as a code owner July 3, 2024 09:00
@vpaturet
vpaturet requested a review from t2gran July 3, 2024 09:01
@vpaturet vpaturet self-assigned this Jul 3, 2024
@t2gran t2gran added this to the 2.6 (next release) milestone Jul 3, 2024
@t2gran t2gran added the !Improvement A functional improvement or micro feature label Jul 3, 2024
@leonardehrenfried leonardehrenfried changed the title Expose stop transfer priority in API Expose stop transfer priority in Transmodel API Jul 4, 2024
@leonardehrenfried
leonardehrenfried self-requested a review July 4, 2024 14:01
.value(
"preferred",
StopTransferPriority.PREFERRED,
"Preferred place to transfer, strongly recommended. NeTEx equivalent is PREFERRED_INTERCHANGE."

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.

Woudl it be possible to use @DocumentedEnum here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you are referring to?

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.

Rather than using Javadoc which is then copied to the GraphQL schema, you can annotate the enum and have only a single place for the documentation, like this:

@Override
public String enumValueDescription() {
return switch (this) {
case RAIL -> "Used for intercity or long-distance travel.";
case COACH -> "Used for long-distance bus routes.";
case SUBWAY -> "Subway or Metro, used for any underground rail system within a metropolitan area.";
case BUS -> "Used for short- and long-distance bus routes.";
case TRAM -> "Tram, streetcar or light rail. Used for any light rail or street level system within a metropolitan area.";
case FERRY -> "Used for short- and long-distance boat service.";
case AIRPLANE -> "Taking an airplane";
case CABLE_CAR -> "Used for street-level cable cars where the cable runs beneath the car.";
case GONDOLA -> "Gondola or suspended cable car. Typically used for aerial cable cars where the car is suspended from the cable.";
case FUNICULAR -> "Used for any rail system that moves on steep inclines with a cable traction system.";
case TROLLEYBUS -> "Used for trolleybus systems which draw power from overhead wires using poles on the roof of the vehicle.";
case MONORAIL -> "Used for any rail system that runs on a single rail.";
case CARPOOL -> """
Private car trips shared with others.
This is currently not specified in GTFS so we use the mode type values 1550-1560 which are in the range of private taxis.
""";
case TAXI -> "Using a taxi service";
};

It's particularly useful if you have the information in both the HTML documentation as well as the schema, so it's usefulness isn't quite so high here.

@leonardehrenfried leonardehrenfried 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.

Since the usefulness of DocumentedEnum is very debatable, I will approve and let you decide if it's a net plus in this case.

@vpaturet

vpaturet commented Jul 5, 2024

Copy link
Copy Markdown
Contributor Author

We can discuss this in the next dev meeting

@vpaturet
vpaturet merged commit 73e93a8 into opentripplanner:dev-2.x Jul 8, 2024
@vpaturet
vpaturet deleted the expose_stop_transfer_priority_in_api branch July 8, 2024 09:42
t2gran pushed a commit that referenced this pull request Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

!Improvement A functional improvement or micro feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants