Modify

Opened 19 months ago

Closed 16 months ago

Last modified 16 months ago

#17100 closed enhancement (fixed)

complain about descriptive names and offer to fix them

Reported by: mkoniecz Owned by: Klumbumbus
Priority: normal Milestone: 19.03
Component: Core validator Version:
Keywords: template_report Cc:

Description (last modified by mkoniecz)

What steps will reproduce the problem?

  1. Create a closed way with amenity=parkink and name=parking
  2. Run validator

What is the expected result?

Validator complains about undesirable descriptive name and proposes to remove it.

What happens instead?

Nothing

Please provide any additional information below. Attach a screenshot if possible.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2018-12-11 00:24:50 +0100 (Tue, 11 Dec 2018)
Build-Date:2018-12-11 02:32:21
Revision:14551
Relative:URL: ^/trunk

Identification: JOSM/1.5 (14551 en) Linux Ubuntu 16.04.5 LTS
Memory Usage: 497 MB / 869 MB (294 MB allocated, but free)
Java version: 1.8.0_191-b12, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: :0.0 1920x1080
Maximum Screen Size: 1920x1080
Dataset consistency test: No problems found

Plugins:
+ OpeningHoursEditor (34535)
+ buildings_tools (34724)
+ continuosDownload (82)
+ imagery_offset_db (34641)
+ measurement (34529)
+ reverter (34552)
+ todo (30306)

Last errors/warnings:
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet

Attachments (1)

invalid parking names.png (51.9 KB) - added by mkoniecz 19 months ago.

Download all attachments as: .zip

Change History (49)

Changed 19 months ago by mkoniecz

Attachment: invalid parking names.png added

comment:1 Changed 19 months ago by mkoniecz

Attachment has history how many such names were present according to the http://taghistory.raifer.tech/ tool

comment:2 Changed 19 months ago by GerdP

This is quite special. Similar problem that I came across: building=house with name=house or shop=* with name=Shop.
Do you think about a test that would flag any name=v where this v also appears for another tag (excluding all other name tags
like name:de or alt_name etc), maybe ignoring the case so that Shop and shop do both match?

comment:3 Changed 19 months ago by mkoniecz

This is quite special. Similar problem that I came across: building=house with name=house or shop=* with name=Shop

Yes, my plan was to propose this obvious and relatively popular case and check whatever this rule type would be considered as OK. And in case of inclusion later propose some other potential matches from my list that are added so often that I am unable to process them on my own.

Do you think about a test that would flag any name=v where this v also appears for another tag (excluding all other name tags
like name:de or alt_name etc)

I worry that it may have many false positives. For start, in my own validator script I already exclude:

  • places (there are places with all kinds of bizarre name, like city in Turkey called Batman - and probably somewhere there is a village actually called Parking)
  • restaurants (restaurants sometimes have deliberately crazy and bizarre names, restaurant named Parking would not be outside what is normal)
  • peaks (some peaks also have truly weird names, I expect the same for other natural features but they are not mapped so often so I still have to run into one)

Sometimes descriptive name indicates that editor got confused (like tourism=attraction, name=hotel) and in such cases simply removing name is not helping.

There are also objects tagged solely with name=parking - for such cases removing names is not helping at all.

Overall I think that it would be safer to start from known popular descriptive names and add more manually rather than start from generic rule and add exclusions. I think that first would achieve the same results with less work and less false positives exposed to mappers. But the second approach is also OK.

Last edited 19 months ago by mkoniecz (previous) (diff)

comment:4 Changed 19 months ago by mkoniecz

Description: modified (diff)

comment:5 Changed 19 months ago by majkaz

I would love to have this: ...Do you think about a test that would flag any name=v ...

implemented like this: use the preset values, and for object (node/way/relation) with tag, for example building, the validator complains about any "preset" value of "building" used for name of the same object.
Just for tourism=attraction I would include all the possible values from presets. Reasoning: first in mobile apps (maps.me / OsmAnd) and gets often misused as catchall. Others might know other suspected tags, where this is the same / similar case.

What I have seen, all the remaining errors fall under the "object has only name and no other tags" test.

If testing for all possible values of all keys, I would set this as secondary test, getting "suspicious names". But this should be possibly an extra an user can disable without disabling the first one.

comment:6 Changed 19 months ago by GerdP

Owner: changed from team to GerdP

I'll think about this. Probably no implementation this month.

comment:7 Changed 17 months ago by mkoniecz

I can provide patch with set of simple rules (amenity=parking + name=parking, leisure=pitch + name=pitch, leisure=pitch + sport=soccer + name=boisko etc).

Is there chance that it would be merged?

comment:8 Changed 17 months ago by GerdP

Hard to say without knowing the patch ;)
The first example occurs quite often, but your other two examples only a few times. I don't think that we should add a test for each dubious combination.

comment:9 Changed 17 months ago by mkoniecz

OK, that no longer would be a good example.

Is 800+ cases like for name=playground/name=Playground enough ( http://overpass-turbo.eu/s/GyP )?

Or 900+ for name=shop/name=Shop ( http://overpass-turbo.eu/s/GyN )?

Though 13k for name=building ( http://overpass-turbo.eu/s/GyR ) certainly qualifies.

Last edited 17 months ago by mkoniecz (previous) (diff)

comment:10 Changed 17 months ago by GerdP

Yes, I think so, too.

comment:11 Changed 17 months ago by mkoniecz

There is also name=kiosk shop=kiosk with 891 uses http://overpass-turbo.eu/s/Gz1

comment:12 Changed 17 months ago by mkoniecz

Questions:

  • Should it go in group "deprecated tagging" or create a new one like "descriptive names"?
  • Is there a better way to ignore case in matches?

Code below is not a patch and is not tested, I just looked at validator rules - though feedback is welcomed

*[name=parking][amenity=parking],
*[name=Parking][amenity=parking]
 {
  throwWarning: tr("{0} descriptive names are unneded", "{0.tag}");
  group: tr("deprecated tagging");
  suggestAlternative: "it is enough to use amenity=parking, name=parking is unnecessary";
  fixRemove: "name";
  assertMatch: "way name=parking amenity=parking";
  assertMatch: "way name=parking amenity=Parking";
  assertMatch: "node name=parking amenity=parking";
  assertMatch: "node name=parking amenity=Parking";
  assertMatch: "relation name=parking amenity=parking";
  assertMatch: "relation name=parking amenity=Parking";
  assertNoMatch: "way name=parking";
}

Last edited 16 months ago by Don-vip (previous) (diff)

comment:13 Changed 17 months ago by mkoniecz

And cemeteries looks like a good candidates for a similar check: http://overpass-turbo.eu/s/Gzr http://overpass-turbo.eu/s/Gzs

comment:14 in reply to:  12 Changed 17 months ago by Klumbumbus

Replying to mkoniecz:

  • Should it go in group "deprecated tagging" or create a new one like "descriptive names"?

I suggest:

*[name=building][building],
...
...
...
*[name=parking][amenity=parking] {
  throwWarning: tr("{0}", "{0.tag}");
  group: tr("descriptive name");
  fixRemove: "name";
  assertMatch: ...
}

(and it should be added in unnecessary.mapcss)


  • Is there a better way to ignore case in matches?

This seems to be a bug. wiki:Help/Styles/MapCSSImplementation states several times that without quotes it is case insensitive.

comment:15 Changed 17 months ago by mkoniecz

@Klumbumbus

Thanks for idea of putting into one rule and directing me to unnecessary.mapcss and to docs that it is case insensitive (I have not tested it, just stupidly assumed that it is case sensitive).

comment:16 Changed 17 months ago by Klumbumbus

I tested it and it doesn't work but I think it used to work in the past.
(edit: ... the case insensitiveness)

Last edited 17 months ago by Klumbumbus (previous) (diff)

comment:17 Changed 17 months ago by mkoniecz

My current plan for failed insensitivity is to:

*[name=parking][amenity=parking],
*[name=playground][leisure=playground],
*[name=shop][shop][shop!=no],
*[name=building][building][building!=no],
*[name=kiosk][shop=kiosk],
*[name=cemetery][amenity=graveyard],
*[name=cemetery][landuse=cemetery],
*[name=cmentarz][amenity=graveyard],
*[name=cmentarz][landuse=cemetery]
 {
  throwWarning: tr("{0}", "{0.tag}");
  group: tr("descriptive name");
  fixRemove: "name";
  assertMatch: "way name=parking amenity=parking";
  assertMatch: "way name=parking amenity=Parking";
  assertMatch: "node name=parking amenity=parking";
  assertMatch: "node name=parking amenity=Parking";
  assertMatch: "relation name=parking amenity=parking";
  assertMatch: "relation name=parking amenity=Parking";
  assertNoMatch: "way name=parking";
  assertMatch: "relation name=PLAYGROUND leisure=playground";
  assertMatch: "node name=PLaYGrOUNd leisure=playground";
  assertNoMatch: "way name=playground";
  assertMatch: "node name=shop shop=whatever";
  assertNoMatch: "node name=shop shop=no";
  assertNoMatch: "way name=shop leisure=playground";
  assertMatch: "way name=building building=yes";
  assertNoMatch: "way building=yes";
  assertMatch: "way name=kiosk building=yes shop=kiosk";
  assertNoMatch: "way name=kiosk building=yes";
  assertMatch: "way name=cemetery amenity=graveyard";
  assertMatch: "way name=cmentarz amenity=graveyard";
  assertMatch: "way name=Cmentarz amenity=graveyard";
  assertNoMatch: "way name=kiosk amenity=graveyard";
}
Last edited 16 months ago by Don-vip (previous) (diff)

comment:18 Changed 17 months ago by mkoniecz

Current proposed validator code below. Now I need to find git mirror (found at https://github.com/openstreetmap/josm ).

*[name=Parking][amenity=parking],
*[name=Playground][leisure=playground],
*[name=Shop][shop][shop!=no],
*[name=Building][building][building!=no],
*[name=Kiosk][shop=kiosk],
*[name=Cemetery][amenity=graveyard],
*[name=Cemetery][landuse=cemetery],
*[name=Cmentarz][amenity=graveyard],
*[name=Cmentarz][landuse=cemetery],
# rules above are temporary workaround for https://josm.openstreetmap.de/ticket/17398#ticket
*[name=parking][amenity=parking],
*[name=playground][leisure=playground],
*[name=shop][shop][shop!=no],
*[name=building][building][building!=no],
*[name=kiosk][shop=kiosk],
*[name=cemetery][amenity=graveyard],
*[name=cemetery][landuse=cemetery],
*[name=cmentarz][amenity=graveyard],
*[name=cmentarz][landuse=cemetery]
 {
  throwWarning: tr("{0}", "{0.tag}");
  group: tr("descriptive name");
  fixRemove: "name";
  assertMatch: "way name=parking amenity=parking";
  assertMatch: "way name=Parking amenity=parking";
  assertMatch: "node name=parking amenity=parking";
  assertMatch: "node name=Parking amenity=parking";
  assertMatch: "relation name=parking amenity=parking";
  assertMatch: "relation name=parking amenity=Parking";
  assertNoMatch: "way name=parking";
  assertMatch: "relation name=PLAYGROUND leisure=playground";
  assertMatch: "node name=PLaYGrOUNd leisure=playground";
  assertNoMatch: "way name=playground";
  assertMatch: "node name=shop shop=whatever";
  assertNoMatch: "node name=shop shop=no";
  assertNoMatch: "way name=shop leisure=playground";
  assertMatch: "way name=building building=yes";
  assertNoMatch: "way building=yes";
  assertMatch: "way name=kiosk building=yes shop=kiosk";
  assertNoMatch: "way name=kiosk building=yes";
  assertMatch: "way name=cemetery amenity=graveyard";
  assertMatch: "way name=cmentarz amenity=graveyard";
  assertMatch: "way name=Cmentarz amenity=graveyard";
  assertNoMatch: "way name=kiosk amenity=graveyard";
}
Last edited 16 months ago by Don-vip (previous) (diff)

comment:20 Changed 17 months ago by mkoniecz

Summary: complain about name=parking or name=Parking for amenity=parkingcomplain about name=parking or name=Parking for amenity=parking [PATCH]

comment:21 Changed 16 months ago by Klumbumbus

Milestone: 19.03

comment:22 Changed 16 months ago by GerdP

@Klumbusbus: Please take over. I thought about an implementation in Java, but that would probably produce too many false positives.

comment:23 Changed 16 months ago by Klumbumbus

I was waiting if #17398 will be fixed first :) but I just noticed Don-vip commented there.

comment:24 Changed 16 months ago by mkoniecz

I closed it by editing wiki and wontfixing it per Don-vip comment.

comment:25 Changed 16 months ago by mkoniecz

I tried moving to regexp but

*[name=~/^parking$/][amenity=parking],
*[name=~/^playground$/][leisure=playground],
*[name=~/^shop$/][shop][shop!=no],
*[name=~/^building$/][building][building!=no],
*[name=~/^kiosk$/][shop=kiosk],
*[name=~/^cemetery$/][amenity=graveyard],
*[name=~/^cemetery$/][landuse=cemetery],
*[name=~/^cmentarz$/][amenity=graveyard],
*[name=~/^cmentarz$/][landuse=cemetery]
 {
  throwWarning: tr("{0}", "{0.tag}");
  group: tr("descriptive name");
  fixRemove: "name";
  assertMatch: "way name=parking amenity=parking";
  assertMatch: "way name=Parking amenity=parking";
  assertMatch: "node name=parking amenity=parking";
  assertMatch: "node name=Parking amenity=parking";
  assertNoMatch: "node name=Parking_with_suffix amenity=parking";
  assertNoMatch: "node name=Megaparking amenity=parking";
  assertMatch: "relation name=parking amenity=parking";
  assertMatch: "relation name=parking amenity=Parking";
  assertNoMatch: "way name=parking";
  assertMatch: "relation name=PLAYGROUND leisure=playground";
  assertMatch: "node name=PLaYGrOUNd leisure=playground";
  assertNoMatch: "way name=playground";
  assertMatch: "node name=shop shop=whatever";
  assertNoMatch: "node name=shop shop=no";
  assertNoMatch: "way name=shop leisure=playground";
  assertMatch: "way name=building building=yes";
  assertNoMatch: "way building=yes";
  assertMatch: "way name=kiosk building=yes shop=kiosk";
  assertNoMatch: "way name=kiosk building=yes";
  assertMatch: "way name=cemetery amenity=graveyard";
  assertMatch: "way name=cmentarz amenity=graveyard";
  assertMatch: "way name=Cmentarz amenity=graveyard";
  assertNoMatch: "way name=kiosk amenity=graveyard";
} 

Causes throwWarning: tr("{0}", "{0.tag}"); to stop working.

comment:26 Changed 16 months ago by Klumbumbus

It works with brackets: *[name=~/^(parking)$/][amenity=parking], but then again not anymore with embedded (?i)

comment:27 Changed 16 months ago by Klumbumbus

Not sure if it is the best solution but *[name][name=~/^(?i)(parking)$/][amenity=parking], does the trick.

Last edited 16 months ago by Klumbumbus (previous) (diff)

comment:28 Changed 16 months ago by mkoniecz

Ready, unit tests now all pass! Thanks for the help!

*[name][name=~/^(?i)(parking)$/][amenity=parking],
*[name][name=~/^(?i)(playground)$/][leisure=playground],
*[name][name=~/^(?i)(shop)$/][shop][shop!=no],
*[name][name=~/^(?i)(building)$/][building][building!=no],
*[name][name=~/^(?i)(kiosk)$/][shop=kiosk],
*[name][name=~/^(?i)(cemetery)$/][amenity=graveyard],
*[name][name=~/^(?i)(cemetery)$/][amenity=cemetery],
*[name][name=~/^(?i)(cmentarz)$/][amenity=graveyard],
*[name][name=~/^(?i)(cmentarz)$/][amenity=cemetery]
 {
  throwWarning: tr("{0}", "{0.tag}");
  group: tr("descriptive name");
  fixRemove: "name";
  assertMatch: "way name=parking amenity=parking";
  assertMatch: "way name=Parking amenity=parking";
  assertMatch: "node name=parking amenity=parking";
  assertMatch: "node name=Parking amenity=parking";
  assertNoMatch: "node name=Parking_with_suffix amenity=parking";
  assertNoMatch: "node name=Megaparking amenity=parking";
  assertMatch: "relation name=parking amenity=parking type=multipolygon";
  assertMatch: "relation name=Parking amenity=parking type=multipolygon";
  assertNoMatch: "way name=parking";
  assertMatch: "relation name=PLAYGROUND leisure=playground type=multipolygon";
  assertMatch: "node name=PLaYGrOUNd leisure=playground";
  assertNoMatch: "way name=playground";
  assertMatch: "node name=shop shop=whatever";
  assertNoMatch: "node name=shop shop=no";
  assertNoMatch: "way name=shop leisure=playground";
  assertMatch: "way name=building building=yes";
  assertNoMatch: "way building=yes";
  assertMatch: "way name=kiosk building=yes shop=kiosk";
  assertNoMatch: "way name=kiosk building=yes";
  assertMatch: "way name=cemetery amenity=graveyard";
  assertMatch: "way name=cmentarz amenity=graveyard";
  assertMatch: "way name=Cmentarz amenity=graveyard";
  assertNoMatch: "way name=kiosk amenity=graveyard";
} 

comment:29 Changed 16 months ago by Klumbumbus

Resolution: fixed
Status: newclosed

In 14884/josm:

fix #17100 - warn about descriptive names (patch by mkoniecz)

comment:30 Changed 16 months ago by Klumbumbus

(I added Spielplatz and Parkplatz)

comment:31 Changed 16 months ago by GerdP

Maybe add a test for building=house + name=house, appears ~ 1100 times

comment:32 Changed 16 months ago by Klumbumbus

name=house and name=House are 7455 together.

comment:33 Changed 16 months ago by Klumbumbus

In 14885/josm:

see #17100 - warn about name=house too

comment:34 Changed 16 months ago by GerdP

highway=* and name=jalan or Jalan is also a good candidate, jalan means road/street

comment:35 Changed 16 months ago by Klumbumbus

Resolution: fixed
Status: closedreopened

comment:36 Changed 16 months ago by GerdP

There are also > 20000 nodes with
emergency=fire_hydrant
fire_hydrant:type=underground
name=地下式消防栓

comment:37 in reply to:  36 Changed 16 months ago by Klumbumbus

Replying to GerdP:

There are also > 20000 nodes with
emergency=fire_hydrant
fire_hydrant:type=underground
name=地下式消防栓

Maybe an import?

comment:38 Changed 16 months ago by GerdP

Yes, possible. It is used in a rather small area. I'll have a look at the changeset and maybe add a comment.

Another candidate:
building=* and name=Rumah
Rumah means house

comment:39 Changed 16 months ago by mkoniecz

fix #17100 - warn about descriptive names (patch by mkoniecz)

Note that not only warns - it also offers to autoremove names.

That was my reason for not including name=house as there may be some value in moving this info to building tag.

Though data added by somebody using name tag is suspect anyway, so throwing it away is probably also OK.

comment:40 Changed 16 months ago by mkoniecz

Yes, possible. It is used in a rather small area. I'll have a look at the changeset and maybe add a comment.

Looks like a yet another import. I commented in https://www.openstreetmap.org/changeset/68120342#map=11/25.0792/121.6027&layers=N (made 19 hours ago).

comment:41 Changed 16 months ago by mkoniecz

(I added Spielplatz and Parkplatz)

Thanks for reviewing, merging and adding other popular descriptive names!

Last edited 16 months ago by mkoniecz (previous) (diff)

comment:42 Changed 16 months ago by Klumbumbus

Ticket #17476 has been marked as a duplicate of this ticket.

comment:43 Changed 16 months ago by Klumbumbus

from #17476:
man_made=silo + name=Silo
http://overpass-turbo.eu/s/H5q - 245 cases in many places across the world

comment:44 Changed 16 months ago by mkoniecz

untested code for entries that IMHO certainly should be included:

*[name][name=~/^(?i)(silo)$/][man_made=silo],
*[name][name=~/^(?i)(silo)$/][building=silo],
*[name][name=~/^(?i)(地下式消防栓)$/][emergency=fire_hydrant],
*[name][name=~/^(?i)(rumah)$/][building=house],

and tests

  assertMatch: "way name=silo man_made=silo";
  assertMatch: "way name=Silo man_made=silo building=silo";
  assertMatch: "way name=Silo building=silo";
  assertNoMatch: "way name=kiosk building=silo";
  assertMatch: "way name=地下式消防栓 emergency=fire_hydrant";
  assertMatch: "way name=Rumah building=house";
  assertMatch: "way name=RuMaH building=house";

I am unsure about

name=House/Rumah on building=yes

BTW, it may be worth moving https://josm.openstreetmap.de/browser/josm/trunk/data/validator/deprecated.mapcss#L456 here (*[name="АЗС"][amenity=fuel],)

highway=* and name=jalan or Jalan is also a good candidate, jalan means road/street

4632 objects :( - see http://overpass-turbo.eu/s/H7x

I am unsure whatever highway=* is not too gready, maybe match on known highway values?

comment:45 Changed 16 months ago by mkoniecz

Summary: complain about name=parking or name=Parking for amenity=parking [PATCH]complain about descriptive names and offer to fix them

comment:46 Changed 16 months ago by Klumbumbus

Owner: changed from GerdP to Klumbumbus
Status: reopenednew

(hm, changing the owner of the ticket without changing the status seems impossible)

comment:47 Changed 16 months ago by Klumbumbus

Resolution: fixed
Status: newclosed

In 14911/josm:

fix #17100, see #17471 - add name=silo|rumah|jalan to descriptive name warning, move АЗС there too, disable autofix for cases where the value of the name tag could be the value of the other tag (manual fix by human required in these cases)

comment:48 Changed 16 months ago by Klumbumbus

I didn't add name=地下式消防栓 as this is only a local problem. All other comments after comment:33 should be considered with r14911.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Klumbumbus.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.