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

Adding ref for railway=subway_entrance #2370

Merged
merged 1 commit into from
Dec 10, 2016

Conversation

kocio-pl
Copy link
Collaborator

@kocio-pl kocio-pl commented Sep 24, 2016

Resolves #645.

I guess we need to wait for a database reload to show also refs, but it's possible to render the name of subway entrances right now (15k+ uses):
a8ne_h8q

UPDATE: probably ref should be available, but is not in .stations - could somebody help me with it?

@nebulon42
Copy link
Contributor

ref is in the database. You need to select it in the query (https://github.com/gravitystorm/openstreetmap-carto/blob/master/project.yaml#L1653) so that you can use it in CartoCSS. I would recommend the same approach as with housenames and housenumbers.

Two questions come to my mind:

  • Why have the font in italics?
  • Why not use the icon colour for the text?

@kocio-pl
Copy link
Collaborator Author

Fixed now! The problem proved to be I needed to add ref also here, otherwise I got an error when trying to show entrances with ref.

I was reusing tram stop settings, including color, but wanted to make it look a bit different in case there is a tram stop around. Italics is a nice way to do it and it hints that it's something less important.

Look at this example:
0f3u upx

Using station-color for labels does not give visible results. If these entrances have the same name (it may still happen) and ref, it could be confusing, don't you think?

@kocio-pl kocio-pl changed the title Adding name for railway=subway_entrance Adding name and ref for railway=subway_entrance Sep 25, 2016
@kocio-pl
Copy link
Collaborator Author

Different styles of rendering primary example, because it have both name and ref:
1)
bnav3uwt
2)
txu krea
3)
wu_nzfhp
4)
hmyvof1a
5)
9svbabhz

Now it's clear why we need darker color for labels. This example is hard (which is good for testing!), because refs are hardly visible because of background elements and only 3) really works. So 5) is out of question for me, but we still may go with italics/normal and station-text/standard black. What do you prefer?

@matthijsmelissen
Copy link
Collaborator

I'm not sure what the name tag on subway entrances is used for? Is it the name of the entrance, or the name of the station? In the latter case, I'm not sure if rendering it makes much sense.

@dieterdreist
Copy link

sent from a phone

Il giorno 26 set 2016, alle ore 12:00, Matthijs Melissen notifications@github.com ha scritto:

I'm not sure what the name tag on subway entrances is used for

name is always referring to the object it is tagged on (i.e. the entrance in this case), although many mappers are not doing it like this;-)

@jojo4u
Copy link

jojo4u commented Sep 26, 2016

Well wiki says: Name of the exit if is known. Not the name of the station

@kocio-pl
Copy link
Collaborator Author

In the updated code I have chosen 1), because confusion with tram stop is not likely (even in the given example it's easy to properly attribute labels to objects) and it's still quite clear without breaking the icon/label colors similarity.

Copy link
Collaborator

@pnorman pnorman left a comment

Choose a reason for hiding this comment

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

I wasn't sure I'd be able to review this because Vancouver's system is elevated, making finding entrance locations much different than an underground system, but I spent some time in Toronto.

Where I saw this having an impact it lead to many duplications of the station name, a bad result. To see if this was just a local mapping style I looked at other cities with Overpass Turbo, and it's common.

@kocio-pl
Copy link
Collaborator Author

Do you propose to show just ref labels? I would be OK with that solution.

However the problem with data still exists: wiki is clear about not using station name, so it's just a common tagging error or wiki needs to be updated to reflect reality.

@pnorman
Copy link
Collaborator

pnorman commented Oct 14, 2016

Do you propose to show just ref labels? I would be OK with that solution.

However the problem with data still exists: wiki is clear about not using station name, so it's just a common tagging error or wiki needs to be updated to reflect reality.

The ref tags I see have the ref of the station or the ref of the lines the station has.

With the data I've seen, I don't see value in rendering any text labels for station entrances.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Oct 14, 2016

Which cities have you tried? With London, Warsaw, Moscow, Tokio or Paris refs look OK. New York has this problem, but the station names have it too.

@matthijsmelissen
Copy link
Collaborator

I see value in rendering station entrance refs. Some metro systems rely heavy on them (and within metro stations, GPS coverage is usually absent).

Hopefully accepting this PR will lead to better data.

@kocio-pl
Copy link
Collaborator Author

You mean only the refs or the names too?

@matthijsmelissen
Copy link
Collaborator

I meant the refs.

@kocio-pl kocio-pl changed the title Adding name and ref for railway=subway_entrance Adding ref for railway=subway_entrance Oct 15, 2016
@kocio-pl
Copy link
Collaborator Author

I have updated this PR to render only refs.

@pnorman
Copy link
Collaborator

pnorman commented Oct 16, 2016

Which cities have you tried? With London, Warsaw, Moscow, Tokio or Paris refs look OK. New York has this problem, but the station names have it too.

New York and Cincinnati airport are the two places with ref on railway=subway_entrance in North America. New York is actually a bit more complicated than I had realized, it doesn't have the ref of the lines at the station, it has the ref of the lines you can reach from that station entrance.

Looking at some other areas, I'm neutral on this PR overall.

@dieterdreist
Copy link

2016-10-16 22:36 GMT+02:00 Paul Norman notifications@github.com:

New York and Cincinnati airport are the two places with ref on
railway=subway_entrance in North America. New York is actually a bit more
complicated than I had realized, it doesn't have the ref of the lines at
the station, it has the ref of the lines you can reach from that
station entrance.

Looking at some other areas, I'm neutral on this PR overall.

refs (like all other tags) refer to the object they are attached to, i.e. a
ref on an entrance should be the ref of this entrance, not the ref of
things that can be reached through this entrance. If you want to tag the
latter you should use a different tag.

@matthijsmelissen
Copy link
Collaborator

I think the color of the label should match the color of the icon (not sure which of both should match).

@kocio-pl
Copy link
Collaborator Author

Are you sure?

We already use different colors for all the stations and stops (station-color + station-text or transportation-icon + transportation-text). I think this is done by design, to make labels (thin lines) be visible more or less like corresponding icons (thick objects).

In case of subway entrance (transportation-icon) I would rather render labels as transportation-text to be consistent wit the rest than use transportation-icon.

@matthijsmelissen
Copy link
Collaborator

matthijsmelissen commented Nov 18, 2016

Are you sure?

I think so - we have been working on making icons and labels match over the past year, and I don't think we should add additional non-matching ones. I thin the stations should be changed as well. See also #1127.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Nov 18, 2016

I think that current design has a lot of sense, but that's not a blocking problem for me, so I've changed the code according to your opinion and now it looks like this (fixed and updated):
tcwqtxa9

@kocio-pl kocio-pl force-pushed the subway_entrance branch 2 times, most recently from b08c832 to 6cda888 Compare December 4, 2016 19:50
@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Dec 4, 2016

The code is updated now and ready to be merged.

@matthijsmelissen
Copy link
Collaborator

Thanks!

@matthijsmelissen matthijsmelissen merged commit e2455db into gravitystorm:master Dec 10, 2016
@kocio-pl kocio-pl deleted the subway_entrance branch December 10, 2016 19:33
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.

6 participants