search

openstreetmap / iD

🆔 The easy-to-use OpenStreetMap editor in JavaScript.
https://www.openstreetmap.org/edit?editor=id
ISC License
3.34k stars 1.2k forks source link

Offer to upgrade generic named features to use proper tagging #5949

Open matkoniecz opened 5 years ago

matkoniecz commented 5 years ago

Some people (especially beginners) use landuse instead of shop tags.

For example https://www.openstreetmap.org/way/435769787 was tagged as landuse=retail name=bakery

iD in that situation should not propose removal of name=bakery

This name should be removed as generic solely from shop=bakery.

I propose to stop suggesting of generic names from landuses - and maybe propose switching to correct object type (preset for bakery may have list of generic names used for it).

selection_001

landuse=retail without name because it was removed is much harder to fix than landuse=retail name=bakery that is really easy to fix, so removing generic names from generic landuses is harmful.

Proposing to switch directly to shop=bakery would be probably even better.

matkoniecz commented 5 years ago

Other example = shop=yes + name=Bakery (or like in my case name=Piekarnia).

BjornRasmussen commented 5 years ago

This would require a list of what generic names should be had as. Perhaps a list of generic names could be added to each preset rather than the names being stored in a single list?

DujaOSM commented 5 years ago

Apologies for hijacking this semi-related issue, but what happened in Update 2.15.0? It seems that generic names warning is removed altogether? What does the "Suspicious Names" validation exactly do now?

I was using this feature a lot, assigning temporary labels to otherwise unnamed landuse relations (Farmland "1", Scrub "6") solely for editing purposes (much easier to build relations this way), and I was warned before commit, so I could remove those names. Now there's no warning at all, so I tend to leave garbage around...

matkoniecz commented 5 years ago

Apologies for hijacking this semi-related issue, but what happened in Update 2.15.0? It seems that generic names warning is removed altogether?

It seems to be present, I just tested with name=hotel

Line has the suspicious name "hotel"

bhousel commented 5 years ago

Apologies for hijacking this semi-related issue, but what happened in Update 2.15.0? It seems that generic names warning is removed altogether? What does the "Suspicious Names" validation exactly do now?

The validation is still there, it's just that the source list has changed to not include names like the temporary ones you were using. We don't want to warn people about "Motel 6" after all.

DujaOSM commented 5 years ago

It seems to be present, I just tested with name=hotel

Well, I tested it with a relation called "farmland", a cemetery called "cemetery", and an orchard called "orchard" and it did not complain. So it seems to be of very limited use. Before the number-as-name validation was introduced, I used special characters such as . , _ / : in order to minimize the impact of potential commit, but the iD warning was great. Can I get my toy back in some form, please? I can't think of many situations when a number would constitute a valid name for anything, and non-alphanumeric characters in particular.

DujaOSM commented 5 years ago

Well, I tested it with a relation called "farmland", a cemetery called "cemetery", and an orchard called "orchard" and it did not complain.

But on the other hand, it did issue a warning

Farmland has the suspicious name "hotel"

so I have my workaround. :)

bhousel commented 5 years ago

TBH I don't know how this issue is still open. If you add the name "bakery" to a landuse, it's totally ok for iD to warn about it.

matkoniecz commented 5 years ago

it's totally ok for iD to warn about it

The problem is that iD is not just warning, it is encouraging mapper to remove it

For example https://www.openstreetmap.org/way/435769787 was tagged as landuse=retail name=bakery

iD in that situation should not propose removal of name=bakery, it is not helpful - the proper fix in that case was to move it to shop=bakery

matkoniecz commented 5 years ago

Note that title is

Do not suggest to remove generic names from landuse

not

Do not warn about generic names on landuse

See for example also https://www.openstreetmap.org/way/605240938#map=19/14.57863/121.11999 or https://www.openstreetmap.org/node/4697920841

1ec5 commented 5 years ago

Perhaps a solution would be to add a second option below it, for removing the landuse tag. Choosing that option would trigger the error about missing a feature tag, which has its own suggestion to choose an appropriate feature tag. That way, we don’t need to fuzzy-match on preset names, which could get messy.

bhousel commented 5 years ago

iD in that situation should not propose removal of name=bakery, it is not helpful - the proper fix in that case was to move it to shop=bakery

Got it - I was indeed confused by the title.

quincylvania commented 4 years ago

That way, we don’t need to fuzzy-match on preset names, which could get messy.

@1ec5 is right that this would be messy.

iD compares place names to discardNames from https://github.com/osmlab/name-suggestion-index/blob/master/dist/filters.json. These are just regular expression that aren't tied to any specific feature types, and some could match multiple, e.g. ^(beer|convenience|corner|general|liquor|variety)(\\sstore)?$. While some like ^bakery$ would be straightforward, I don't see a great way to implement this fix with a high success rate given the current data structure.

In any case, I don't see this fix as critical. It would be nice to have, but how often does a feature's preset mismatch with its name anyway?

bhousel commented 4 years ago

In any case, I don't see this fix as critical. It would be nice to have, but how often does a feature's preset mismatch with its name anyway?

Yeah we've gone back and forth a few times on this issue, but I think what they are really asking for is to check if name=thing happens to match a preset for shop=thing (or amenity=thing etc), and if so to offer to switch to that other preset and remove the name. That's pretty easy I think.

For generic names, iD currently suggests to "remove the name" which is kind of not the correct fix, but at least calls attention to the issue.

1ec5 commented 4 years ago

check if name=thìng happens to match a preset forshop=thing(oramenity=thing` etc)

Seems like users would be less likely to tag name=thing with the main workflow in v3, which forces the user to choose a preset before they’d have a chance to name the feature.

quincylvania commented 4 years ago

For generic names, iD currently suggests to "remove the name" which is kind of not the correct fix, but at least calls attention to the issue.

It's true this doesn't fit in all cases, but it's useful when a feature is correctly tagged yet has a redundant, generic name. Like amenity=drinking_water with name=water fountain.