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

Filter vector tiles stops by current service week #6003

Merged

Conversation

leonardehrenfried
Copy link
Member

@leonardehrenfried leonardehrenfried commented Aug 9, 2024

Summary

As requested by @fpurcell this PR allows the filtering of the vector tiles stop layer by the current service week.

It introduces a new concept into the vector tiles: a layer filter, which allows you remove results from the regular set of geometries.

Issue

Relates to #4879

Unit tests

✔️

Documentation

Autogenerated & Javadoc.

@leonardehrenfried leonardehrenfried requested a review from a team as a code owner August 9, 2024 08:13
@leonardehrenfried leonardehrenfried marked this pull request as draft August 9, 2024 08:14
Copy link

codecov bot commented Aug 9, 2024

Codecov Report

Attention: Patch coverage is 67.24138% with 19 lines in your changes missing coverage. Please review.

Project coverage is 69.79%. Comparing base (dd2d0df) to head (253bb17).
Report is 63 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
...ipplanner/ext/vectortiles/layers/LayerFilters.java 60.00% 8 Missing ⚠️
...xt/vectortiles/layers/stops/StopsLayerBuilder.java 0.00% 3 Missing ⚠️
...pplanner/apis/gtfs/datafetchers/QueryTypeImpl.java 0.00% 3 Missing ⚠️
...ntripplanner/apis/gtfs/generated/GraphQLTypes.java 62.50% 2 Missing and 1 partial ⚠️
...s/gtfs/support/filter/PatternByDateFilterUtil.java 80.00% 1 Missing ⚠️
...ntripplanner/inspector/vector/LayerParameters.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             dev-2.x    #6003   +/-   ##
==========================================
  Coverage      69.78%   69.79%           
- Complexity     17362    17367    +5     
==========================================
  Files           1964     1964           
  Lines          74388    74403   +15     
  Branches        7625     7626    +1     
==========================================
+ Hits           51911    51928   +17     
+ Misses         19833    19831    -2     
  Partials        2644     2644           

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

@leonardehrenfried leonardehrenfried marked this pull request as ready for review September 5, 2024 19:36
@leonardehrenfried
Copy link
Member Author

@optionsome optionsome self-requested a review September 10, 2024 09:28
@leonardehrenfried
Copy link
Member Author

@optionsome @t2gran I move the class PatternByServiceDatesFilter into the core model and separate the GraphQL-specific parts from it. This is now ready for review.

@leonardehrenfried leonardehrenfried added the IBI Developed by or important for IBI Group label Sep 12, 2024
doc/user/examples/ibi/portland/router-config.json Outdated Show resolved Hide resolved
import org.opentripplanner.transit.service.TransitService;

/**
* Utility methods for instantiating a {@link PatternByServiceDatesFilter}/
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
* Utility methods for instantiating a {@link PatternByServiceDatesFilter}/
* Utility methods for instantiating a {@link PatternByServiceDatesFilter}.

Copy link
Member

Choose a reason for hiding this comment

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

I think the / was a mistake. but if not, feel free to ignore this suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was. Applied.

Copy link
Member

Choose a reason for hiding this comment

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

Why am I still seeing the / here? What caused this 🤔 ?

@leonardehrenfried
Copy link
Member Author

Since I was struggling to attract reviewers, I added one more parameter to Stop#routes in order to not split up the work into too many PRs.

import org.opentripplanner.transit.service.TransitService;

/**
* Utility methods for instantiating a {@link PatternByServiceDatesFilter}/
Copy link
Member

Choose a reason for hiding this comment

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

Why am I still seeing the / here? What caused this 🤔 ?

@leonardehrenfried
Copy link
Member Author

leonardehrenfried commented Sep 17, 2024

@optionsome I think I might have made a mistake when merging/rebasing. It's fixed now.

Co-authored-by: Joel Lappalainen <[email protected]>
@t2gran t2gran added this to the 2.7 (next release) milestone Sep 18, 2024
@leonardehrenfried leonardehrenfried merged commit dc56145 into opentripplanner:dev-2.x Sep 20, 2024
5 checks passed
t2gran pushed a commit that referenced this pull request Sep 20, 2024
@leonardehrenfried leonardehrenfried deleted the trimet-vector-layer branch September 20, 2024 11:57
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.

4 participants