Improve handling of preference `validator.UnconnectedWays.way_way_distance`

[GerdP]

Description for original ticket "Remove or improve test "Unconnected natural lands and landuses":
I fear I don't understand what this test is about. It collects ways whith keys landuse or natural but it will ignore all unconnected ways with key landuse and most of those with natural as they return true for method concernsArea()
So far I only found the tag natural=ridge which produces a useful warning.
Possible candidates like natural=tree_row or natural=cliff are explicitely excluded in the code. No idea why.
Typos like natural=woter will also produce a warning because concernsArea() returns false.

If preference validator.UnconnectedWays.way_way_distance is changed to a value > 0 (e.g. 5) this test will also produce "Other" messages about two landuse/natural areas which are very close (< 5m) but not connected. No idea where this is relevant.

[GerdP]

No idea where this is relevant.

Ah, it can be used to find "unmapped" areas between other areas.
Example: ​https://www.openstreetmap.org/way/101185578 and ​https://www.openstreetmap.org/way/12249128
Or nodes which might be merged, like node 2245347926 and node 105292216

[GerdP]

special case with missleading Connected way end node near other way (2) messages

[GerdP]

My findings so far:
When I change the preference validator.UnconnectedWays.way_way_distance from 0 to e.g. 1 (meter) I see some new messages like
Connected way end node near other way
Example: ​https://www.openstreetmap.org/node/2245347794
I really wonder if anybody understands what that means and what makes it special compared to nearby node
​https://www.openstreetmap.org/node/2245347793
which produces a different message Way node near other way
My current understanding is that the first node is used by multiple ways, the second is only used by one. In both cases the node is a candidate for action J Join node to way or N Move node onto way

I wondered if we should simply suggest on of these actions in the message? Something like
Consider to connect node with way
until I found the case in attachment:20057-bad.osm​
You don't understand the message until you zoom in to find a culvert with a length of 4 centimeters (!)
This is probably a candidate for a new warning

[skyper]

Nice findings. Wonder how a way with width=0.04 m can be called a track.

+1 for a new warning. Think linear features like the second example should be treated differently than areas in the first example.

1. I do not find a layer in the linear example. Why do I not get a crossing way warning?
   • Think it is a problem with downloaded area as adding a node to the track north of the waterway inside downloaded area with show the expected warning. Think this example should always be warned about and not only inside downloaded area. -> new ticket?
2. The major problem, I see, is the length of the way. How about a test about length of linear ways in general and having a variable validator.way_min_length to set the minimum allowed value instead of validator.UnconnectedWays.way_way_distance.

[GerdP]

In fact the message from UnconnectedWays is wrong as the way is connected via the short culvert. I've just found a fix for that.
I see a Crossing highway/waterway warning without changing anything. Not sure why you get a different result.

[GerdP]

Reg. short linear ways: I think it difficult to produce a general warning. At least highway=steps are often very short and still useful. I guess there are tunnel=culvert with lengths < 1m in real world, e.g. between barrier=ditch

[skyper]

Strange, now I get the crossing way warning, too.

We have this test with default set to 0.0 or is it 1e7. validator.way_min_length could have the same default value. Additionally, another ignore list could be used.

For messages, how about Short way length or Way length under …?

[GerdP]

I think this can be done in mapcss using waylength(), right?

[GerdP]

attachment:20057-beta.patch​ (well more work in progress than beta)

• fixes the bug shown in attachment:20057-bad.osm​ and adds a unit test for it
• changes the messages produced by the test so that the name is used as a group, e.g. "Unconnected Highways" or "Unconnected Waterways"

[skyper]

Replying to GerdP:

I think this can be done in mapcss using waylength(), right?

You are right. I did not understand how setting variables in mapcss validator rules works and how can I make those user configurable. Or does this only work with styles?

[GerdP]

Did not think about variables. Anyway, please create a new ticket if you think this will not produce too many false positives.

[GerdP]

If I got that right right the setting validator.UnconnectedWays.way_way_distance should be ignored when looking at power objects. I am not a power expert but I think it finds only false postives, e.g. nodes n3887078076 or n1734250675. My understanding is that it would be a bad idea to connect the power lines.

[GerdP]

For highway it helps to find errors like n370755789 in attachment:20057-highway.osm​ but it's also likely to find false positives like ways under bridges (n286333584) because layer is only considered for unconnected end nodes so far. I'll try to fix that first.
It also finds a rather strange case of parallel ways: w575642793 w351460405 which is not detected by other tests, but it produces lots of warnings for the same problem.
Without the fixes implemented in attachment:20057-beta.patch​ it also finds the error in attachment:20057-highway-2.osm​
There are quite a lot of those cases which I would call something like "impossible sharp angle for possibe travel route".
In other words: The test finds lots of real problem cases but the message texts are rather useless and severity is too low.

[GerdP]

For railway it finds far too many false positives near railway=switch without the patch and still too many with it. Also some toys rails in gardens or parks. I looked at many of the cases and found no real problem, so - like with power this is of no use.

[GerdP]

For waterway there are typically lots dubious findings near waterway=weir as well as waterway=river nodes which are close to the enclosing waterway=riverbank (or vice versa). Also many messages for crossing waterways which are also found by CrossingWays. So I think validator.UnconnectedWays.way_way_distance is also of no use with waterway.

[skyper]

In general, I do not know if there is a policy how to handle results of different test. Are all test in core supposed to be one unit or should they all work on their own.

Does it make sense to treat power, high-/rail-/waterway all individually and exclude them from the general test?

Replying to GerdP:

If I got that right right the setting validator.UnconnectedWays.way_way_distance should be ignored when looking at power objects. I am not a power expert but I think it finds only false postives, e.g. nodes n3887078076 or n1734250675. My understanding is that it would be a bad idea to connect the power lines.

Think this depends on the tags of the pole/tower but your examples seem to be false positives. Wonder if one of the power lines should have a layer. At least the information which line crosses above the other is missing, right now.

[GerdP]

Replying to skyper:

In general, I do not know if there is a policy how to handle results of different test. Are all test in core supposed to be one unit or should they all work on their own.

Does it make sense to treat power, high-/rail-/waterway all individually and exclude them from the general test?

We have various tests in core which all work on their own. The java source UnconnectedWays.java implements 5 independent tests which share a common code base:

UnconnectedHighways
UnconnectedRailways
UnconnectedWaterways
UnconnectedNaturalOrLanduse
UnconnectedPower

In the past, this was different, see r6515.

Replying to GerdP:

If I got that right right the setting validator.UnconnectedWays.way_way_distance should be ignored when looking at power objects. I am not a power expert but I think it finds only false postives, e.g. nodes n3887078076 or n1734250675. My understanding is that it would be a bad idea to connect the power lines.

Think this depends on the tags of the pole/tower but your examples seem to be false positives. Wonder if one of the power lines should have a layer. At least the information which line crosses above the other is missing, right now.

Maybe. I don't know if I ever saw power lines crossing like that in real world, but I don't look at them that often ;)