Modify

Opened 3 months ago

Closed 4 weeks ago

#19572 closed enhancement (fixed)

[patch] Complain about nodes with lanes=*

Reported by: anonymous Owned by: team
Priority: normal Milestone: 20.09
Component: Core validator Version:
Keywords: lanes node Cc:

Description

There are about 8k instances where the key lanes is used on a node, while it should only be used on a way.
Perhaps validator can warn about this?

https://overpass-turbo.eu/s/WlD

Attachments (0)

Change History (16)

comment:1 Changed 3 months ago by taylor.smock

I've got that in the Kaart validator: https://github.com/KaartGroup/KaartValidator/blob/master/kaart.clingstone.validator.mapcss#L120

node[/^.*(lanes|turn).*$/],
node[/^.*(surface)/][!/^(traffic_calming)$/],
node[/^.*(maxspeed|maxweight|maxheight|maxwidth).*$/][!/^(traffic_sign|parking|barrier|traffic_calming)$/][!highway=speed_camera] {
  throwError: tr("Nodes typically don''t have {0}", "{0.key}");
  fixRemove: "{0.key}";
  assertMatch: "node lanes=3";
  assertNoMatch: "node name=Me";
}

TBH, it would probably be useful generally as well.

comment:2 Changed 3 months ago by skyper

It is useful but only for a limited number of tags on nodes, like barrier=border_control and barrier=toll_booth but most of the time it is an tagging mistake.

comment:3 in reply to:  2 ; Changed 3 months ago by taylor.smock

Replying to skyper:

It is useful but only for a limited number of tags on nodes, like barrier=border_control and barrier=toll_booth but most of the time it is an tagging mistake.

I presume you are talking about something like barrier=toll_booth + lanes=* on a node?
It looks like (technically) lanes can go on the barrier=toll_booth, but there are only 35 instances of that on the planet ( https://overpass-turbo.eu/s/Wna ). barrier=border_control + lanes=* has 19 instances ( https://overpass-turbo.eu/s/Wnb ). Honestly, it would be better to split ways and add the lane information to the road anyway. But it is (technically) correct.

So something like this:

node[/^.*lanes.*$/][barrier!~/^(border_control|toll_booth)$] {
  throwError: tr("Nodes typically don''t have {0}", "{0.key}");
  fixRemove: "{0.key}";
  assertMatch: "node lanes=3";
  assertNoMatch: "node lanes=3 barrier=toll_booth";
}

Note: instance totals are nodes, ways, and relations, not just nodes.

comment:4 in reply to:  3 Changed 3 months ago by skyper

Replying to taylor.smock:

Replying to skyper:

It is useful but only for a limited number of tags on nodes, like barrier=border_control and barrier=toll_booth but most of the time it is an tagging mistake.

I presume you are talking about something like barrier=toll_booth + lanes=* on a node?
It looks like (technically) lanes can go on the barrier=toll_booth, but there are only 35 instances of that on the planet ( https://overpass-turbo.eu/s/Wna ). barrier=border_control + lanes=* has 19 instances ( https://overpass-turbo.eu/s/Wnb ). Honestly, it would be better to split ways and add the lane information to the road anyway. But it is (technically) correct.

I fully agree but see it as an intermediate step and once tagged in detail you can change it to 1 or remove the tag.

comment:5 Changed 3 months ago by Klumbumbus

I think lanes on junction=roundabout and highway=mini_roundabout nodes is acceptable.

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

comment:6 in reply to:  5 ; Changed 3 months ago by Famlam

Seems the lanes wiki page is outdated ;) (It still forbids it's use on nodes)
Also leisure=slipway seems to use it a few times. (And it's also used for some indoor leisure=track nodes)

Perhaps there's a mapCSS equivalent for the following, which catches the majority (5k out of 8k) of the cases and is guaranteed safe?

  node["lanes"](if:count_tags()==1)({{bbox}});

(and if so, it might be worthwhile to also add the following, which occur much more often than lanes:)

  node["surface"](if:count_tags()==1)({{bbox}});
  way["surface"](if:count_tags()==1)({{bbox}});

comment:7 in reply to:  6 Changed 3 months ago by skyper

Replying to Famlam:


Perhaps there's a mapCSS equivalent for the following, which catches the majority (5k out of 8k) of the cases and is guaranteed safe?

Have a look at number_of_tags() under Evalexpressions.

comment:8 Changed 3 months ago by skyper

Same tags should not be considered so probably the pseudo class :tagged is what is needed here, e.g.:

node!:tagged["lanes"] { }

Edit: Above does not work as we have the tag lanes=*. So no chance to use the pseudo class.

As long as the list of allowed tags for nodes is low I'd prefer to have them explicitly in the rule as it will catch more cases. This is even true for surface=*.

Last edited 3 months ago by skyper (previous) (diff)

comment:9 Changed 3 months ago by Klumbumbus

surface=* could be on lots of POI mapped as node. leisure=pitch, traffic_calming=bump, amenity=parking, ford=yes, ...

comment:10 in reply to:  9 Changed 3 months ago by skyper

Replying to Klumbumbus:

surface=* could be on lots of POI mapped as node. leisure=pitch, traffic_calming=bump, amenity=parking, ford=yes, ...

Ok, using :tagged for surface won't produce false positives. For lanes=* on nodes, I think the number of exceptions are low.

I start with an alphabetically sorted list here, please, modify the comment to add more or to update the list.

comment:11 Changed 3 months ago by Famlam

Ignoring all clearly wrong cases (i.e. what's a bank with lanes?), and the ones you already listed, lanes is also used (and provides additional information not otherwise stored) on:

  • 85 barriers of 6 types (excluding border_control and toll_booth in the counting)
  • 59 junction of 2 types (yes and roundabout, although I would argue it then also for circular)
  • 44 traffic_calmings of 9 different types
  • 27 public_transports of 2 different types (platform and stop_position; I guess the number of platforms in a bigger station?)
  • 1 amenity=parking_entrance
  • 1 leisure=bowling_alley (the number of bowling lanes?)

The wiki only lists it as a valid key for:

  • junction=roundabout (but unclear if also valid for nodes)
  • leisure=track (should be used as ways, but sometimes nodes are used when indoor)
  • barrier=toll_booth

However, I would argue that it can be useful information for the others listed without much room for misinterpretation, and thus doesn't necessarily have to be deleted. I.e., if I would make the list, I would make it a bit more tolerant:

  • barrier=*
  • ford=*
  • highway=mini_roundabout
  • junction=*
  • leisure=*
  • traffic_calming=*

This would allow 455 cases of `lanes` on a node, roughly 6%.

comment:12 in reply to:  11 ; Changed 3 months ago by skyper

Replying to Famlam:

Ignoring all clearly wrong cases (i.e. what's a bank with lanes?),

A ATM/Bank with several drive-through lanes. I personally like the presence of shop=alcohol on the service=drive-through wiki page.

Thanks for the stats.
I would still diversify leisure=* but the rest I like so far.

comment:13 in reply to:  12 Changed 3 months ago by Famlam

No clue how to make a good patch file (I used a separate .validator.mapcss file), but hopefully this helps if someone can copy-paste?

Add to this test

/* 19572 */
*[surface][number_of_tags()=1],
*[lanes][number_of_tags()=1] {
  throwWarning: tr("incomplete object: only {0}", "{0.key}");
  group: tr("missing tag");
  set .only_one_tag;
  assertMatch: "node surface=asphalt";
  assertMatch: "node lanes=1";
  assertMatch: "way surface=asphalt";
  assertNoMatch: "way surface=asphalt highway=residential";
  assertNoMatch: "node lanes=1 barrier=toll_booth";
  assertNoMatch: "node surface=asphalt traffic_calming=hump";
}

Add to this test

/* 19572 */
node[lanes][!barrier][!ford][highway!=mini_roundabout][!junction][leisure!~/^(bowling_alley|slipway|track)$/][!traffic_calming]!.only_one_tag {
  throwWarning: tr("{0} on suspicious object", "{0.key}");
  group: tr("suspicious tag combination");
  assertMatch: "node amenity=bank lanes=1";
  assertNoMatch: "node barrier=toll_booth";
  assertNoMatch: "node lanes=1"; /* Dealt with separately*/
}

note: throwWarning and group are existing lines

Last edited 3 months ago by Famlam (previous) (diff)

comment:14 Changed 3 months ago by Klumbumbus

Milestone: 20.08
Summary: Complain about nodes with lanes=*[patch] Complain about nodes with lanes=*

comment:15 Changed 7 weeks ago by Klumbumbus

Milestone: 20.0820.09

comment:16 Changed 4 weeks ago by Klumbumbus

Resolution: fixed
Status: newclosed

In 17050/josm:

fix #19572 - Add more warnings about lanes and surface on suspicious objects (based on patch by Famlam)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
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.