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

Remove operator text label rendering for amenity=vending_machine #3965

Merged
merged 2 commits into from
Jan 31, 2020

Conversation

jeisenbe
Copy link
Collaborator

@jeisenbe jeisenbe commented Nov 5, 2019

Fixes #3946

Changes proposed in this pull request:

  • Remove operator= text label rendering for amenity=vending_machine features

Explanation:

The operator tag is often incorrect and rarely helpful for these features.

Searching for all in Wales with operator= finds a few more useful examples, including: "Amazon" , "Transport for Wales", "Knolton Farmhouse Cheese", "4CG", and "Newport Bus".

But many more operator tags are unhelpful: "Ceredigion Council", "Pembrokeshire Coast National Park Authority", "Pembrokeshire County Council", "Conwy Council", "Bangor University" x2, "Campus Catering", "Caerphilly County Borough Council".

The example given in issue #3946 is node 4329279576 which has the operator is "Stadt Göttingen", meaning "Göttingen City", so rendering this is not useful.

It would make the most sense to stop rendering the operator tag for vending machines.

Test rendering with links to the example places:

https://www.openstreetmap.org/#map=19/51.52810/9.94055
Before
vending-before

After
vending-after

@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 5, 2019

I'm not convinced that this is really useful change. Currently there are some names that you don't know, after the change you will not know anything about these machines, so it seems to me like a step back.

@Adamant36
Copy link
Contributor

Adamant36 commented Nov 5, 2019

If this removes rendering for public transport ticket's then I wouldn't do it. As sometimes vending machines for multiple operators are pretty close to each other. Like here you have them both for BART and Caltrain.

I don't think the original justification of "This is also how OsmAnd+ and JOSM do it by default" stated in the original issue is a compelling one either. In the original issue the name tag "Neues Rathaus 1 - Außenstellplätze," which translates to "New Town Hall 1 - outdoor parking spaces" that seems like a descriptive use of the name tag. Which goes against the naming rule. Getting rid of the operator tag might just encourage more of that type of wrong (IMHO) tagging. If anything I think rendering of the name label should be removed from OsmAnd+ and JOSM (although I'm not suggesting it).

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 5, 2019 via email

@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 6, 2019

This is general problem, I don't know why do you limit it only to the ticket machines.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 7, 2019

@kocio-pl, I don't fully understand this comment:

after the change you will not know anything about these machines

We currently show a 3 unique icons depending on the value of vending:

  1. amenity=vending_machine + vending=parking_tickets -
  2. amenity=vending_machine + vending=public_transport_tickets -
  3. amenity=vending_machine + vending=excrement_bags -

Over 2/3rd lack an operator= tag, so the icon is all that we show in most cases.

this is general problem, I don't know why do you limit it only to the ticket machines.

As suggested before, the only seemingly useful examples of showing the operator= tag would be in the case of vending=public_transport_tickets at a train station where there is more than one operator of these machines. This is an unusual situation. Most of the time, the operator for vending=public_transport_tickets is just the same as the operator of the whole train or bus station, e.g. "Transport for Wales" above. This isn't helpful, since there is no other type of public transport ticket available.

Similarly, while there are several different public transport operators in Singapore, all of the ticket vending machines work for all the operators, so the name of the operator is irrelevant: they all use the same electronic card.

Knowing the operator of a public transport ticket machines is only very rarely useful. But since it could be useful on those rare situations (like at a major train station where you can transfer between intercity trains and local commuter trains / buses), we could keep rendering the operator tag for all of them.

For parking lots, you don't have a choice of which operator to use, so I can't think of a situation where it would be helpful to know the name of the parking lot operator.

Similarly, there's no use knowing the operator of a vending device for excrement bags.

Since the operator tag is unhelpful the vast majority of the time, I am suggesting stopping rendering it entirely for amenity=vending_machine, for simplicity and clarity, but we can instead make an exception and just show it for public_transport_tickets if there is consensus that this is beneficial.

@Prince-Kassad
Copy link

As sometimes vending machines for multiple operators are pretty close to each other.

Which means the operator won't get rendered anyway due to lack of space, even on the highest zoom level z19.

We won't prevent apps from processing the operator tag, but trying to render them on the map when that usually fails due to space issues is rather worthless.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 9, 2019

Over 2/3rd lack an operator= tag, so the icon is all that we show in most cases.

One of the goals of this style is to encourage users to add data and we do it by giving some feedback (rendering them in some way). Usually objects have a name, but using some other tag, like operator or ref, might make sense instead (or together - roads use both name and the ref for example).

Maybe we need to rethink what label we prefer for sub-cases of vending machines, if you believe they are different enough, but I don't like removing the feedback what is there and what is missing. Using high enough zoom level and low rendering priority can be right tools for this problem.

Similarly, there's no use knowing the operator of a vending device for excrement bags.

OSM is very broad ecosystem and I believe nobody can see all the cases, so I'm cautious with general arguments like what is "important" or "there is no use".

This isn't helpful, since there is no other type of public transport ticket available.

I agree that it isn't helpful for showing the difference in specific place. But for example comparing how it works in different cities is possible then.

For parking lots, you don't have a choice of which operator to use, so I can't think of a situation where it would be helpful to know the name of the parking lot operator.

That's why using ref might be better choice here. Besides - they are sometimes painted on the ground.

@jragusa
Copy link
Contributor

jragusa commented Nov 10, 2019

I agree to remove operator label to vending=parking_tickets because the operator is almost the same in a given city and it does not add critical information on the map. However, I am not against to render ref tag if this help people to found the correct machine.

@jeisenbe
Copy link
Collaborator Author

Using high enough zoom level and low rendering priority can be right tools for this problem

We don't currently have a way to do this, except by moving amenity=vending_machine to the low-priority amenity and text layers. This would mean that the icon would be blocked by any other text or label.

@kocio-pl, is that what you are suggesting, or would this require solving #3880 first (to have a way to prioritize features by initial zoom level)

@kocio-pl
Copy link
Collaborator

I understand #3880 as a big, radical tuning which is independent from any small-scale problem solving, like in this ticket, so I don't think we need to wait for that.

@jeisenbe
Copy link
Collaborator Author

I pushed a new commit which restores the operator=* rendering for vending=public_transport_tickets.

This will still solve issue #3946 by removing the operator= text label from parking_ticket vending machines.

Copy link
Collaborator

@kocio-pl kocio-pl left a comment

Choose a reason for hiding this comment

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

I think removing any label is wrong and I'm against this. There should be one of them visible - name, ref, operator etc. (in case of roads we even use two of them). However this is not a blocker for me.

@jeisenbe
Copy link
Collaborator Author

However this is not a blocker for me.

I don't understand what this means.

@kocio-pl
Copy link
Collaborator

This means that I don't agree, but leave it to you to consider. There are big problems and small problems, and this is the later for me.

@jeisenbe
Copy link
Collaborator Author

Anyone else have thoughts on this PR?

The operator label would be removed vending machines with vending=excrement_bags and vending=parking_tickets, since the name of the operator is not of interest to the general map user in these cases.

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 agree with removing the operator text label, and have tested this.

Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

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

Not tested but i agree with this change. Showing the operator on vending machines for parking tickets or excrement bags is distinctly non-helpful for the map user in all cases i have looked at. Even in the theoretical case of two parkings next to each other and ambiguity which machine belongs to which parking the operator will often not help because for the parking we display the name and it would incentivize abusing the operator tag for something else.

For public transport tickets i see the potential use - but also that this is often distinctly non-helpful for someone without in depth local knowledge. The better solution would be to have consistent tagging of network=* or similar generic information on what specific kinds of tickets are sold at a certain machine and use that for varying the rendering. Overall i would be in support of not showing the operator for those either.

@jeisenbe
Copy link
Collaborator Author

Ok, we have 2 approving reviews as well as myself in favor, and @kocio-pl thinks we ought to render the ref= or name= instead, but that does not prevent us from reaching consensus to remove the operator= labels from these two features at this time, since it was stated: #3965 (comment) "I don't agree, but leave it to you to consider. There are big problems and small problems, and this is the later for me."

@jeisenbe
Copy link
Collaborator Author

I plan to merge this tomorrow, unless there are new opposing views or other changes requested.

@jeisenbe jeisenbe merged commit 78c12c3 into gravitystorm:master Jan 31, 2020
@jeisenbe jeisenbe deleted the vending branch January 31, 2020 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Label for vending_machine=parking_ticket uses uncommon value selection
7 participants