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

Use station tag value in pmap:kind_detail and include tram stops in POIs #166

Closed
wants to merge 8 commits into from

Conversation

eikes
Copy link
Contributor

@eikes eikes commented Oct 26, 2023

Use station tag value in pmap:kind_detail to differentiate subway / light_rail / train stations

Include railway=tram_stop in POIs

This PR fixes #165

See:
https://wiki.openstreetmap.org/wiki/Key:station
https://wiki.openstreetmap.org/wiki/Tag:railway%3Dtram_stop

Copy link
Collaborator

@nvkelso nvkelso left a comment

Choose a reason for hiding this comment

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

Should min_zoom carry by the new station kind_detail value?

Same comment as other Pr, let's set an explicate min_zoom on new tram_stop, too. Probably zoom 15 or 16?

@eikes
Copy link
Contributor Author

eikes commented Oct 29, 2023

Should min_zoom carry by the new station kind_detail value?

Same comment as other Pr, let's set an explicate min_zoom on new tram_stop, too. Probably zoom 15 or 16?

To match bus stops, I set the tram_stop to zoom 16. Regular train stations should be as important as visible as libraries right? So I set it to 13 as well. Subway and light rail at 14 seemed appropriate. WDYT @nvkelso ?

@eikes
Copy link
Contributor Author

eikes commented Oct 29, 2023

I also added railway: halt

@bdon
Copy link
Member

bdon commented Oct 31, 2023

Do we need to use a different tagging scheme once we have a single POI that is part of multiple transit modes?

https://tilezen.readthedocs.io/en/latest/layers/#poi-properties-only-on-kindstation

@eikes
Copy link
Contributor Author

eikes commented Oct 31, 2023

These transit networks are distinct. I don't think that these nodes exist. As far as I can tell, these stations are mapped as distinct nodes as well, even when they are in the same building.

@eikes
Copy link
Contributor Author

eikes commented Nov 1, 2023

I changed the zoom for the train (implicit if absent) and light_rail stations to minzoom 10 match the tilezen definitions:

https://github.com/tilezen/vector-datasource/blob/master/yaml/pois.yaml#L1948-L1953

(I also set subway to the same, because it is not explicitly set in the tilezen definition.)

I set tram_stop and halt to minzoom 16:

https://github.com/tilezen/vector-datasource/blob/master/yaml/pois.yaml#L1958-L1965

Copy link

sonarqubecloud bot commented Nov 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

43.5% 43.5% Coverage
0.0% 0.0% Duplication

@eikes
Copy link
Contributor Author

eikes commented Nov 4, 2023

I finally fixed all the code formatting issues!

@bdon
Copy link
Member

bdon commented Nov 5, 2023

Thanks for fixing the formatting!

Ultimately the nature of this tag is to power a different icon, and the one we have in Tilezen looks like this:

station

This is used indiscriminately for light rail, subway, tram.

Committing to a certain tag means we need to stick with that tag going forward for all styles. @eikes can you give more background on how your style plans to use this tag?

@bdon
Copy link
Member

bdon commented Nov 5, 2023

Oh , we also have kind=tram_stop

https://tilezen.readthedocs.io/en/latest/layers/

tram_stop

@nvkelso open to guidance on whether we want to stick with pure Tilezen or pursue a different tagging scheme, previous linked issue: tilezen/vector-datasource#532

@eikes
Copy link
Contributor Author

eikes commented Nov 8, 2023

I also wanted to mention, that there are in fact nodes which have two conflicting kinds at the same time:

https://www.openstreetmap.org/node/2498920310

found via

https://overpass-turbo.eu/s/1D92

@nvkelso
Copy link
Collaborator

nvkelso commented Nov 9, 2023

👋 Sorry for the delay, I was traveling.

Expanding the docs that @bdon linked to:

In Tilezen, any station POI can have a set of is_* boolean flags indicating whether the station has any routes of the given type. These are: is_train, is_subway, is_light_rail, is_tram, corresponding to the above *_routes. This is provided as a convenience for styling.

The booleans are calculated by walking the transit relations that go thru the station node (reading properties on the node is not reliable), and would be new logic to add. But walking relations in Planetiler is "free", so "easy"? ;)

This is because for most general usage maps it's just "something train, something on rails", while for a transit map style you'd want custom icons (and even grouped icons).

For the cases where the type of station is ambiguous... station should imply some sort of "rails" station, and the aerialway station should get a distinct new kind value (eg aerialway_station since it's icon should be different).

@bdon
Copy link
Member

bdon commented Nov 9, 2023

+1 to walking relations to determine station types.

Since we're designing for our MapLibre style, is there even a way right now to show multiple "stacked" icons for a multi-modal transit station? (I know Apple does this.) The best I've seen is using the SDF font hack to show multiple "icons" that are actually glyphs in a fontstack.

@eikes
Copy link
Contributor Author

eikes commented Nov 9, 2023

That's true! You can't really put two icons on the same node anyways. But as a designer of a map I might be more interested in one kind of node than the other.

@bdon
Copy link
Member

bdon commented Feb 7, 2024

Sorry, I have to close this PR for now - please keep the issue open so we can revisit it! We are focusing on achieving parity with the Tilezen project in 2024, and because this feature expands beyond the tagging scheme of Tilezen it's difficult to incorporate into the design right now. At a future date we may build beyond the Tilezen schema or offer a configuration-driven way to add extra tags.

@bdon bdon closed this Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Differentiate train station types
3 participants