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 ID to mapper error messages #5363

Merged

Conversation

leonardehrenfried
Copy link
Member

Summary

Similarly to #4993 this improves the error messages when an entry from a stops.txt has an incorrect location_type, for example when you try to generate a transfer from a station to a station.

cc @NVBWSeifert

@leonardehrenfried leonardehrenfried added GTFS Related to import of GTFS data Technical Debt Skip Changelog NVBW Nahverkehrsgesellschaft Baden-Württemberg labels Sep 18, 2023
@leonardehrenfried leonardehrenfried requested a review from a team as a code owner September 18, 2023 14:19
@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Patch coverage: 36.36% and project coverage change: +0.02% 🎉

Comparison is base (72dcf51) 66.45% compared to head (0f4588a) 66.48%.

Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #5363      +/-   ##
=============================================
+ Coverage      66.45%   66.48%   +0.02%     
- Complexity     15235    15242       +7     
=============================================
  Files           1787     1787              
  Lines          69325    69323       -2     
  Branches        7319     7319              
=============================================
+ Hits           46072    46089      +17     
+ Misses         20778    20757      -21     
- Partials        2475     2477       +2     
Files Changed Coverage Δ
...a/org/opentripplanner/gtfs/mapping/StopMapper.java 85.36% <0.00%> (-4.38%) ⬇️
...rg/opentripplanner/gtfs/mapping/StationMapper.java 68.00% <50.00%> (-11.32%) ⬇️

... and 4 files with indirect coverage changes

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

Copy link
Contributor

@vesameskanen vesameskanen left a comment

Choose a reason for hiding this comment

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

Code looks good. However, I did some tests:

  • stop_times refers to station -> error is thrown
  • stop's parent station is stop -> no error
  • transfer between stations -> no error
  • station has parent stop -> no error

Can you double check that station mapper error throwing works as expected?

@leonardehrenfried
Copy link
Member Author

Does it work with current dev-2.x?

@vesameskanen
Copy link
Contributor

dev-2.x behaves the same way. I just found it odd that it seems difficult to hit that validation check. There are no regressions - just better quality code - so I will approve this PR.

@vesameskanen vesameskanen self-requested a review September 20, 2023 05:44
@leonardehrenfried leonardehrenfried merged commit 42e3aae into opentripplanner:dev-2.x Sep 21, 2023
@leonardehrenfried leonardehrenfried deleted the mapper-errors branch September 21, 2023 08:03
@t2gran t2gran added this to the 2.5 (next release) milestone Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GTFS Related to import of GTFS data NVBW Nahverkehrsgesellschaft Baden-Württemberg Skip Changelog Technical Debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants