Modify

Opened 4 months ago

Closed 5 weeks ago

Last modified 5 weeks 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 4 months ago.

Download all attachments as: .zip

Change History (49)

Changed 4 months ago by mkoniecz

Attachment: invalid parking names.png added

comment:1 Changed 4 months ago by mkoniecz

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

comment:2 Changed 4 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 4 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 4 months ago by mkoniecz (previous) (diff)

comment:4 Changed 4 months ago by mkoniecz

Description: modified (diff)

comment:5 Changed 4 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 4 months ago by GerdP

Owner: changed from team to GerdP

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

comment:7 Changed 8 weeks 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 8 weeks 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 8 weeks 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 8 weeks ago by mkoniecz (previous) (diff)

comment:10 Changed 8 weeks ago by GerdP

Yes, I think so, too.

comment:11 Changed 8 weeks ago by mkoniecz

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

comment:12 Changed 8 weeks 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 8 weeks ago by Don-vip (previous) (diff)

comment:13 Changed 8 weeks 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 8 weeks 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 8 weeks 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 8 weeks 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 8 weeks ago by Klumbumbus (previous) (diff)

comment:17 Changed 8 weeks 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 8 weeks ago by Don-vip (previous) (diff)

comment:18 Changed 8 weeks 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 8 weeks ago by Don-vip (previous) (diff)

comment:20 Changed 8 weeks 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 8 weeks ago by Klumbumbus

Milestone: 19.03

comment:22 Changed 6 weeks 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 6 weeks ago by Klumbumbus

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

comment:24 Changed 6 weeks ago by mkoniecz

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

comment:25 Changed 6 weeks 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 6 weeks ago by Klumbumbus

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

comment:27 Changed 6 weeks ago by Klumbumbus

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

Last edited 6 weeks ago by Klumbumbus (previous) (diff)

comment:28 Changed 6 weeks 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 6 weeks ago by Klumbumbus

Resolution: fixed
Status: newclosed

In 14884/josm:

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

comment:30 Changed 6 weeks ago by Klumbumbus

(I added Spielplatz and Parkplatz)

comment:31 Changed 6 weeks ago by GerdP

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

comment:32 Changed 6 weeks ago by Klumbumbus

name=house and name=House are 7455 together.

comment:33 Changed 6 weeks ago by Klumbumbus

In 14885/josm:

see #17100 - warn about name=house too

comment:34 Changed 6 weeks ago by GerdP

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

comment:35 Changed 6 weeks ago by Klumbumbus

Resolution: fixed
Status: closedreopened

comment:36 Changed 6 weeks ago by GerdP

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

comment:37 in reply to:  36 Changed 6 weeks 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 6 weeks 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 6 weeks 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 6 weeks 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 6 weeks ago by mkoniecz

(I added Spielplatz and Parkplatz)

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

Last edited 6 weeks ago by mkoniecz (previous) (diff)

comment:42 Changed 6 weeks ago by Klumbumbus

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

comment:43 Changed 6 weeks 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 5 weeks 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 5 weeks 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 5 weeks 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 5 weeks 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 5 weeks 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.