[Patch] Power line checks won't report existing non-power nodes

[francois.lacombe]

Hello

I found out that current power line checks won't report a power line passing by an exiting node of a highway, for instance.

Here the sketch:
I draw a power line and instead of inadvertently connecting it to a highway with a new node, I inadvertently click on an existing node of this highway.
The highway's node will remain unchanged, no new tag, so it won't be reported in "node without power" check nor "Node connects a power line or cable with an object".

Those checks are currently done in the Java class PowerLines.java. I didn't studied it in detail yet but I bet it would be possible to move part of it to mapcss like

way[power=line] > node[!power],
  throwWarning: tr("{0} node without power");
}

If correct, this would allow to strip a bit of specific code and make validation more efficient.

Regarding connections between power lines and highways or waterways, I'm not sure we can achieve them in pure mapcss. Let me know.

Best regards

[GerdP]

I see. You don't get a warning on upload, only when you do a full check. So I think this is a bug in class PowerLines, possibly a regression of r17111.

[francois.lacombe]

Thank you, I indeed get the warning with a full check.

I didn't noticed we already moved from mapcss to java code, so I'm not sure the opposite move is welcome.
By the way r17111 shows what was done before about intersections with building with pure mapcss.

[GerdP]

Seems the problem is a regression of #23397. If you set preference validator.partial.removeIrrelevant to false you also get the messages on upload. I'll dig deeper into this...

[GerdP]

The problem is that the check in class PowerLines creates error messages which do not contain the power line way. Same problem occurs when you draw a new highway=* using e.g. an existing power=pole node.
I see different solutions:
1) Change class PowerLines so that it adds the corresponding way
2) Add a filter for those error types which should never be treated as irrelevant, e.g. the errors from class PowerLines using the code 2501 or 2502.

In both cases the question is how many more tests we have where this problem occurs and how to find them...

[francois.lacombe]

Can I have 1) and 2) please? :)

More seriously, 1) first and let 2) for a more global cleanup if needed

[GerdP]

Patch for test PowerLines so that it adds the related parent objects to the error node

[gaben]

Hmm, I have to test it but looks good.

[GerdP]

Thinking again about it maybe the unrelatedParents should only be collected when nodeInLineOrCable is true, else this creates a lot of work for the garbage collector.

[GerdP]

improved patch

[gaben]

I've just done a quick test of the validation and so far, so good. Hopefully, I will have the capacity to check the code tomorrow.

[gaben]

Hm, I could make the PowerLines test 8 times faster on my dataset. New ticket?

I ran the plain validator on power=line or power=minor_line in Hungary overpass output. Before 9.6s, after 1.2s.

[GerdP]

Yes, please. If you are satisfied with the result I'll commit the 2nd patch.

[gaben]

Take a look at this, I smoothed a bit out and added a test.

I'll open a separate ticket for the performance part. See #24876.

[GerdP]

I first thought about coding it similar to your version but I don't like the idea that the code first selects nodes based on some filters and later tries to determine why the node was select using repeated code with - hopefully - the same filters. This is much harder to read and to maintain. So I favour my solution with the maps, even if they require a bit more memory.

[gaben]

Ok, go ahead. I tested and I'm satisfied with the results.

[GerdP]

In 19619/josm:

Fix #24851: Power line checks won't report existing non-power nodes

• add the related parent objects to the error message so that they are not considered as unrelated when a partial validation is done on upload
• unit test by gaben (thanks!)

[francois.lacombe]

Hello and thank you guys for such a quick fix!