-
Notifications
You must be signed in to change notification settings - Fork 1.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
Extract parking lots from NeTEx feeds #5946
Extract parking lots from NeTEx feeds #5946
Conversation
21d9acd
to
70af97e
Compare
70af97e
to
0ca9720
Compare
@@ -184,6 +190,7 @@ public NetexEntityIndex(NetexEntityIndex parent) { | |||
this.tariffZonesById = new HierarchicalVersionMapById<>(parent.tariffZonesById); | |||
this.brandingById = new HierarchicalMapById<>(parent.brandingById); | |||
this.timeZone = new HierarchicalElement<>(parent.timeZone); | |||
this.parkings = new HashSet<>(parent.parkings); |
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.
Use HierarchicalMapById
eabbf6c
to
aff04b9
Compare
aff04b9
to
5525701
Compare
The serialization id must be bumped due to a change in the Graph object |
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 |
I have now added the switch to ignore the parking data. |
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.
This looks very well worked out. Just a question and a minor issue.
public record KeyValue(String key, Object value) { | ||
public static KeyValue kv(String key, Object value) { | ||
return new KeyValue(key, value); | ||
} | ||
public static KeyValue kv(String key, FeedScopedId value) { |
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'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.
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 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:
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.
src/main/java/org/opentripplanner/standalone/config/buildconfig/NetexConfig.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Henrik Abrahamsson <127481124+habrahamsson-skanetrafiken@users.noreply.github.com>
@vpaturet NeTEx parking is now ignored by default. |
Summary
This parses parking information from NeTEx XML feeds and inserts them into the already existing OTP model for parking data.
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