Modify

Opened 5 years ago

Closed 5 years ago

#10825 closed enhancement (fixed)

add validator warning for tags on isolated nodes, which should be part of a way

Reported by: Klumbumbus Owned by: Klumbumbus
Priority: normal Milestone: 14.12
Component: Core validator Version:
Keywords: Cc:

Description (last modified by Klumbumbus)

I suggest to add the following validator warning:

node:unconnected[barrier=entrance],
node:unconnected[entrance],
node:unconnected[traffic_calming],
node:unconnected[highway=passing_place],
node:unconnected[highway=mini_roundabout],
node:unconnected[highway=motorway_junction],
node:unconnected[highway=turning_loop],
node:unconnected[highway=turning_circle],
node:unconnected[highway=traffic_signals],
node:unconnected[highway=crossing],
node:unconnected[crossing],
node:unconnected[railway=crossing],
node:unconnected[railway=level_crossing],
node:unconnected[railway=buffer_stop],
node:unconnected[noexit],
node:unconnected[waterway=dam],
node:unconnected[waterway=weir],
node:unconnected[waterway=waterfall],
node:unconnected[amenity=ferry_terminal],
node:unconnected[mountain_pass=yes] {
  throwWarning: tr("{0} must be connected to a way", "{1.tag}");
}

(partly based on ticket:9576#comment:5)

Any opinions about this, or more tags to add to the list?

Attachments (0)

Change History (20)

comment:1 Changed 5 years ago by Don-vip

Milestone: 14.12

Railway=crossing

Good idea :)

comment:2 Changed 5 years ago by Klumbumbus

railway=level_crossing
railway=buffer_stop
noexit
highway=passing_place
mountain_pass=yes
barrier=entrance
waterway=dam
waterway=weir
waterway=waterfall
amenity=ferry_terminal

for ford there is aleady a more advanced validator test.

comment:3 Changed 5 years ago by Klumbumbus

Description: modified (diff)

comment:4 Changed 5 years ago by bastiK

+1, can you add it?

comment:5 in reply to:  4 Changed 5 years ago by Klumbumbus

Replying to bastiK:

+1, can you add it?

Yes. I wanted to wait if more tags come.

comment:6 Changed 5 years ago by Klumbumbus

Resolution: fixed
Status: newclosed

In 7793/josm:

fix #10825 - add validator warning for tags on isolated nodes, which should be part of a way

comment:8 Changed 5 years ago by Klumbumbus

Yes, I also just noticed it. This is maybe related to #10164.

comment:9 Changed 5 years ago by Don-vip

In 7796/josm:

see #10825 - handle dataset properly in mapcss tagchecker verification of assertions

comment:10 in reply to:  8 Changed 5 years ago by Don-vip

Replying to Klumbumbus:

This is maybe related to #10164.

Same issue yes, looking to it.

comment:11 Changed 5 years ago by Don-vip

In 7798/josm:

see #10825 - always create nodes with coordinates

comment:12 Changed 5 years ago by aceman

Great idea, thanks!

I suggest also:
highway=stop
highway=give_way
public_transport=stop_position

Not sure about some more of the barrier=*, e.g. barrier=gate (I think there already is some warning about those).

comment:13 in reply to:  12 ; Changed 5 years ago by Klumbumbus

Resolution: fixed
Status: closedreopened

Replying to aceman:

Great idea, thanks!

I suggest also:
highway=stop
highway=give_way
public_transport=stop_position

OK

Not sure about some more of the barrier=*, e.g. barrier=gate (I think there already is some warning about those).

Yes, you're right. We can remove barrier=entrance again, since there is already a more advanced test.

We should add barrier=gate to this (isolated nodes) test, since it should never be isolated but is often on the highway and no linear barrier=* is drawn (so it wouldn't fit in the other java test which I mentioned above).

I will check, which values of barrier=* we should add to the isolated nodes test.

comment:14 in reply to:  13 ; Changed 5 years ago by Don-vip

Replying to Klumbumbus:

Yes, you're right. We can remove barrier=entrance again, since there is already a more advanced test.

Could this test be converted to mapcss?

comment:15 in reply to:  13 Changed 5 years ago by Klumbumbus

Replying to Klumbumbus:

I will check, which values of barrier=* we should add to the isolated nodes test.

  • gate
  • lift_gate
  • swing_gate
  • toll_booth
  • turnstile
  • full-height_turnstile
  • motorcycle_barrier
  • rope
  • sally_port
  • spikes
  • stile
  • sump_buster
  • border_control
  • bump_gate
  • bus_trap
  • cattle_grid
  • chain
  • cycle_barrier
  • hampshire_gate
  • height_restrictor
  • debris

comment:16 in reply to:  14 Changed 5 years ago by Klumbumbus

Replying to Don-vip:

Replying to Klumbumbus:

Yes, you're right. We can remove barrier=entrance again, since there is already a more advanced test.

Could this test be converted to mapcss?

Is a mapcss test the preferred way? (if yes, why? faster? syntax easier to understand?)

It could be replaced by two mapcss test. the isolated nodes test of this ticket and the following:

way[!barrier] > node[barrier=entrance] {
  throwWarning: tr("{0} must be part of a way with {1}", "{0.tag}", "{0.key}");
}

comment:17 Changed 5 years ago by Don-vip

The isOutsideDownloadArea() stuff would be missing, do we need to add a new pseudoclass?

It would not be faster but easier to maintain: all tags-based tests defined at a single place.

comment:18 in reply to:  17 Changed 5 years ago by Klumbumbus

Replying to Don-vip:

The isOutsideDownloadArea() stuff would be missing

The isOutsideDownloadArea() is more useful for the "way terminates on area" warning. For the barrier=entrance warning this is not useful because:

  • I think it's not possible to download an isolated node out of the downloaded area.
  • If you move an isolated node with barrier=entrance out of the downloaded area (or create it there) it is an error again and should be reported (currently it is not!)
  • If you download a way, which has a node with barrier=entrance out of the downloaded area and you delete the way it is an error again and should be reported (currently it is not!)

comment:19 Changed 5 years ago by Klumbumbus

I was wrong. My proposed test:

way[!barrier] > node[barrier=entrance] {
  throwWarning: tr("{0} must be part of a way with {1}", "{0.tag}", "{0.key}");
}

fails when the node with barrier=entrance is connected to a way with barrier=* and a way with e.g. highway=* (because only one parent is accessible) and fails also for the example in #9544. So the isOutsideDownloadArea() thing is after all usefull for the barriers test, so I think we should just simply keep it as is.

comment:20 Changed 5 years ago by Klumbumbus

Resolution: fixed
Status: reopenedclosed

In 7807/josm:

fix #10825 - add some more values for isolated nodes test, remove barrier=entrance because there is already a more advanced test

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.