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.

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

add StopInPattern in GTFS GraphQL schema #6206

Open
wants to merge 5 commits into
base: dev-2.x
Choose a base branch
from

Conversation

miklcct
Copy link
Contributor

@miklcct miklcct commented Oct 29, 2024

Summary

This adds a field called stopsInPattern in the Pattern model for the GTFS GraphQL API.

It provides information which cannot be accessed by the existing API, in particular, the board / alight types of stops in a pattern.

Issue

None yet.

Unit tests

The data fetcher code is not tested at all.

I have added a test for the model constructions.

Documentation

Added in GraphQL.

Note that NULL is used for a cancelled PickDrop to be consistent with existing API. It is not possible to change this without breaking backwards compatibility.

Copy link

codecov bot commented Oct 29, 2024

Codecov Report

Attention: Patch coverage is 57.14286% with 12 lines in your changes missing coverage. Please review.

Project coverage is 69.79%. Comparing base (684b841) to head (dcb5e51).
Report is 1 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
...ripplanner/apis/gtfs/datafetchers/PatternImpl.java 14.28% 6 Missing ⚠️
...ipplanner/apis/gtfs/datafetchers/StoptimeImpl.java 37.50% 4 Missing and 1 partial ⚠️
...nner/apis/gtfs/datafetchers/StopInPatternImpl.java 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             dev-2.x    #6206   +/-   ##
==========================================
  Coverage      69.79%   69.79%           
- Complexity     17786    17788    +2     
==========================================
  Files           2017     2019    +2     
  Lines          76042    76056   +14     
  Branches        7781     7782    +1     
==========================================
+ Hits           53073    53084   +11     
- Misses         20264    20267    +3     
  Partials        2705     2705           

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

@miklcct miklcct marked this pull request as ready for review October 29, 2024 15:17
@miklcct miklcct requested a review from a team as a code owner October 29, 2024 15:17
@optionsome optionsome marked this pull request as draft November 7, 2024 15:31
@miklcct
Copy link
Contributor Author

miklcct commented Nov 14, 2024

@optionsome We discussed before about the use of Pattern in the API and your use case is to show timetables in the app, which you don't want to take real time stop skipping into account (it modifies the pattern in our internal model).

Are you actually showing the real time timetable (with updated times) on the website, or just the scheduled time? How do you deal with the case if a trip is real time diverted?

@miklcct
Copy link
Contributor Author

miklcct commented Nov 14, 2024

We may need to verify the current API behaviour by making a data set with patterns differing in only the board and alight actions.

@miklcct
Copy link
Contributor Author

miklcct commented Nov 15, 2024

I am republishing this PR for discussion. Our existing GTFS GraphQL API distinguish between patterns which differ only in pickup / drop off and they are listed separately in Stop.stoptimesForPatterns

@miklcct miklcct marked this pull request as ready for review November 15, 2024 10:18
@optionsome
Copy link
Member

I am republishing this PR for discussion. Our existing GTFS GraphQL API distinguish between patterns which differ only in pickup / drop off and they are listed separately in Stop.stoptimesForPatterns

I just checked this and looks like you are correct.

@@ -901,6 +901,8 @@ type Pattern implements Node {
semanticHash: String
"List of stops served by this pattern"
stops: [Stop!]
"List of stops with pickup / dropoff type served by this pattern"
stopsInPattern: [StopInPattern!]!
Copy link
Member

Choose a reason for hiding this comment

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

What do you need this for? Is it just for the index? Isn't it straightforward to compute the index yourself, first item in the list is 0 and so on?

@@ -2172,6 +2174,16 @@ type StopGeometries {
googleEncoded: [Geometry]
}

type StopInPattern {
"NULL means that the stop is cancelled from the pattern."
dropoffType: PickupDropoffType
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
dropoffType: PickupDropoffType
dropOffType: PickupDropoffType

"NULL means that the stop is cancelled from the pattern."
dropoffType: PickupDropoffType
indexInPattern: Int!
pattern: Pattern!
Copy link
Member

Choose a reason for hiding this comment

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

Do you need the pattern? Seems like you already have it.

@t2gran t2gran added this to the 2.7 (next release) milestone Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants