Skip to content

Extract parking lots from NeTEx feeds - #5946

Merged
leonardehrenfried merged 33 commits into
opentripplanner:dev-2.xfrom
leonardehrenfried:netex-parking
Jul 29, 2024
Merged

Extract parking lots from NeTEx feeds#5946
leonardehrenfried merged 33 commits into
opentripplanner:dev-2.xfrom
leonardehrenfried:netex-parking

Conversation

@leonardehrenfried

@leonardehrenfried leonardehrenfried commented Jul 4, 2024

Copy link
Copy Markdown
Member

Summary

This parses parking information from NeTEx XML feeds and inserts them into the already existing OTP model for parking data.

image

It also improves the capabilities to debug parking in the debug client.

If you want to see example data for the Italian region of South Tyrol: https://transmodel.api.opendatahub.com/netex/parking

Unit tests

I added a few tests for the mappers but there is a lot of code that does wiring inside the NetexModule. I'm not sure how much I should test that.

Documentation

I only added Javadoc but I feel we should document this somewhere. https://docs.opentripplanner.org/en/latest/Netex-Norway/ seems like a good place to document but it's pretty old and more of a tutorial rather than reference document.

cc @rcavaliere @clezag

@leonardehrenfried leonardehrenfried added +Sandbox This will be implemented as a Sandbox feature +NeTEx This issue is related to the Netex model/import. labels Jul 4, 2024
@leonardehrenfried
leonardehrenfried requested a review from a team as a code owner July 4, 2024 12:43
@codecov

codecov Bot commented Jul 4, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 70.77922% with 45 lines in your changes missing coverage. Please review.

Project coverage is 69.65%. Comparing base (c3c1ea0) to head (ed6b165).
Report is 55 commits behind head on dev-2.x.

Files Patch % Lines
.../inspector/vector/vertex/VertexPropertyMapper.java 0.00% 17 Missing ⚠️
...ripplanner/netex/mapping/VehicleParkingMapper.java 84.61% 5 Missing and 1 partial ⚠️
.../routing/vehicle_parking/VehicleParkingSpaces.java 0.00% 6 Missing ⚠️
...org/opentripplanner/inspector/vector/KeyValue.java 20.00% 3 Missing and 1 partial ⚠️
...ipplanner/netex/loader/parser/SiteFrameParser.java 55.55% 3 Missing and 1 partial ⚠️
...org/opentripplanner/netex/mapping/NetexMapper.java 63.63% 4 Missing ⚠️
...anner/graph_builder/module/StreetLinkerModule.java 0.00% 0 Missing and 1 partial ⚠️
...ntripplanner/netex/config/NetexFeedParameters.java 92.85% 0 Missing and 1 partial ⚠️
...anner/netex/loader/parser/NetexDocumentParser.java 80.00% 0 Missing and 1 partial ⚠️
...eet/model/vertex/VehicleParkingEntranceVertex.java 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             dev-2.x    #5946    +/-   ##
===========================================
  Coverage      69.64%   69.65%            
- Complexity     17138    17186    +48     
===========================================
  Files           1937     1945     +8     
  Lines          73747    73909   +162     
  Branches        7546     7562    +16     
===========================================
+ Hits           51362    51478   +116     
- Misses         19752    19798    +46     
  Partials        2633     2633            

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

this.tariffZonesById = new HierarchicalVersionMapById<>(parent.tariffZonesById);
this.brandingById = new HierarchicalMapById<>(parent.brandingById);
this.timeZone = new HierarchicalElement<>(parent.timeZone);
this.parkings = new HashSet<>(parent.parkings);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Use HierarchicalMapById

@leonardehrenfried
leonardehrenfried requested review from t2gran and removed request for t2gran July 4, 2024 14:17
@leonardehrenfried leonardehrenfried added this to the 2.6 (next release) milestone Jul 12, 2024
@vpaturet vpaturet added the +Bump Serialization Id Add this label if you want the serialization id automatically bumped after merging the PR label Jul 15, 2024
@vpaturet

Copy link
Copy Markdown
Contributor

The serialization id must be bumped due to a change in the Graph object

@vpaturet

vpaturet commented Jul 15, 2024

Copy link
Copy Markdown
Contributor

I have checked with Norwegian data, the parking data do not get imported, probably because of the hierarchical structure of the NeTEx dataset (shared stop file + line-specific files). When mapping the parking in org.opentripplanner.netex.NetexBundle#vehicleParkings, there is no local value in the hierarchical map, all keys are present in the parent hierarchical map.
Maybe you could follow the same pattern as for Entrance or StopPlace?
From a design perspective it is also probably better to define a Parking entity under transit/model/site, so that you can follow the same approach as with the other SiteFrame entities.

@leonardehrenfried

Copy link
Copy Markdown
Member Author

image

The very latest version imports the Norway NeTEx feed without errors.

@leonardehrenfried

Copy link
Copy Markdown
Member Author

I have now added the switch to ignore the parking data.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks very well worked out. Just a question and a minor issue.

public static KeyValue kv(String key, Object value) {
return new KeyValue(key, value);
}
public static KeyValue kv(String key, FeedScopedId value) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm a bit unfamiliar with this part of the code. Could you help me understand what is this KeyValue class used for? And why do we want to stringify the FeedScopedId instead of using the object itself as the value?

I would like the parameter and the property to be marked @Nullable if nulls are expected.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I wrote a bit of JavaDoc but here is the explanation:

This is a class that contains values that are then serialized by the vector tiles library and used in the Debug UI. Normally, you would use the values for styling but we mainly just display them in popups, like this:

image

Since the vector tiles are by design a very efficient data format they don't support any complex data types so FeedScopedId is silently ignored. Since I kept forgetting this, I created a little helper that convert the id to a string.

Comment thread src/main/java/org/opentripplanner/standalone/config/buildconfig/NetexConfig.java Outdated
leonardehrenfried and others added 3 commits July 23, 2024 15:55
Co-authored-by: Henrik Abrahamsson <127481124+habrahamsson-skanetrafiken@users.noreply.github.com>
@leonardehrenfried

Copy link
Copy Markdown
Member Author

@vpaturet NeTEx parking is now ignored by default.

@leonardehrenfried
leonardehrenfried merged commit 475ea53 into opentripplanner:dev-2.x Jul 29, 2024
t2gran pushed a commit that referenced this pull request Jul 29, 2024
t2gran pushed a commit that referenced this pull request Jul 29, 2024
@leonardehrenfried
leonardehrenfried deleted the netex-parking branch July 29, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

+Bump Serialization Id Add this label if you want the serialization id automatically bumped after merging the PR +NeTEx This issue is related to the Netex model/import. +Sandbox This will be implemented as a Sandbox feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants