-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Keep min transfer filter is not local to group-by-filters #5436
Keep min transfer filter is not local to group-by-filters #5436
Conversation
c462279
to
3cff98c
Compare
3cff98c
to
b6b68d0
Compare
I think the elevation snapshot test fails because of this:
Anyone seen this before? Note! this happens in the test setup. |
src/main/java/org/opentripplanner/framework/collection/CollectionUtils.java
Outdated
Show resolved
Hide resolved
...anner/routing/algorithm/filterchain/filter/RemoveDeletionFlagForLeastTransfersItinerary.java
Outdated
Show resolved
Hide resolved
The stacktrace is not the root of the problem but I also cannot figure out what it is so I recommend to remove the test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only noticed typos.
I will disable it for now - then if someone want to take a look they can. |
Co-authored-by: Leonard Ehrenfried <[email protected]>
…ch fails for no good reason
6bf289b
to
277d83c
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #5436 +/- ##
=============================================
+ Coverage 66.85% 66.87% +0.02%
- Complexity 15655 15679 +24
=============================================
Files 1817 1818 +1
Lines 70160 70240 +80
Branches 7383 7392 +9
=============================================
+ Hits 46904 46972 +68
- Misses 20798 20810 +12
Partials 2458 2458 ☔ View full report in Codecov by Sentry. |
Summary
Sometimes a group-by-filter removes the itinerary with the smallest number of transfers. This is not wanted, since few transfers is important to most users. Because, of this we have a filter witch reverse the removal of an itinerary by any of the group-by-filters if the it removes the best option with respect to transfers. The problem is that this filter do not inspect why the itinerary is removed. The reversal should only be applied to the group-by-filter tags, not other filters.
Issue
No issue.
This build on #5433, ready for review when #5433 is merged.
Unit tests
Updated.
Documentation
There are no config for this feature; Hence only JavaDoc to update.
Changelog
This is a minor bug fix.
Bumping the serialization version id
The serialization is not affected.