Modify

Opened 13 months ago

Closed 3 months ago

Last modified 6 weeks ago

#20378 closed defect (fixed)

[Patch] No Warnings on Crossings

Reported by: Hb--- Owned by: GerdP
Priority: normal Milestone: 21.12
Component: Core validator Version: latest
Keywords: Cc:

Description

What steps will reproduce the problem?

  1. Download a region which has a way tagged waterway=stream.
  2. Draw a new area over it and tag it with man_made=bridge.
  3. Draw a new way over the bridge and tag it with highway=primary.
  4. Click Upload data…

What is the expected result?

Get warnings for Crossing highway/waterway and Crossing highway/way.

What happens instead?

Get only a warning for Unnamed ways.

Please provide any additional information below. Attach a screenshot if possible.

If the waterway is modified before a proper highway/waterway warning appears. So this example fails with a new waterway.

The bridge/highway part is an offspring from #20352 comment 4.

The objects do not have any intersecting nodes.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-01-11 14:58:01 +0100 (Mon, 11 Jan 2021)
Build-Date:2021-01-12 02:30:54
Revision:17456
Relative:URL: ^/trunk

Attachments (2)

20378ValidateUploadHook.patch (1.9 KB) - added by Hb--- 13 months ago.
20378ValidateUploadHook-2.patch (1.8 KB) - added by GerdP 4 months ago.
corrected patch (one quote was missing and paths caused trouble)

Download all attachments as: .zip

Change History (15)

comment:1 Changed 13 months ago by GerdP

  • The validator that is executed for upload only looks at modified/new objects. A few tests collect data next or around the modified/new data, but not all. Esp. not the CrossingWays test. If you want full validation you have to run Validator.
  • Up to now we don't have a test for ways crossing man_made=bridge. Might be added, the current wiki seems to recommend that ways over the bridge should share nodes with the man_made=bridge way.

comment:2 in reply to:  1 Changed 13 months ago by Hb---

Replying to GerdP:
The patch changes the message of the dialog to clarify that only a subset of the current data is validated.

comment:3 Changed 13 months ago by Hb---

Patch version 2 replaces the former text

The following are results of automatic validation. Try fixing these, but be careful (don`t destroy valid data). When in doubt ignore them.<br>
When you cancel this dialog, you can find the entries in the validator side panel to inspect them.

with

The Data Validator partially checked the objects to be uploaded and found some problems. Try fixing them, but do not harm valid data. When in doubt ignore the findings.<br>
You can see the findings in the Validator Results panel too. Further checks on all data can be started from that panel.

Changed 13 months ago by Hb---

comment:4 Changed 13 months ago by GerdP

TBH: I think I've never read this text before :O
Not sure if the new text will help to avoid the misunderstandings.

I agree that it is not be obvious to users that the validation which is performed on upload is different to that which is performed by pressing "Shift+V" or clicking the "Validation" button with an empty selection.
It is also difficult to describe what exactly the differences are. Sometimes important problems are not found, sometimes false positives are reported.
In general the upload check is meant to be quick, so that there is no big delay between starting the upload and getting a reaction in form of a dialog. In rare cases this delay is already big, esp. with very complex multipolygon relations if they are completely downloaded. It would probably be a good idea to show a progress indicator after 1 second or so.
An unpleasant scenario is a long edit session with thousands of changes that gets slow because of memory shortage and a crash because the upload validations require too much memory for spatial tests.

Still, I think it would be good to perform tests more thoroughly when a selection of elements is tested. CrossingWays is really fast, I'll check if we can always execute it and just filter the results.

comment:5 Changed 5 months ago by Don-vip

Summary: No Warnings on Crossings[Patch] No Warnings on Crossings

comment:6 Changed 4 months ago by GerdP

Milestone: 21.10
Owner: changed from team to GerdP
Status: newassigned

working on this, see #20680

Changed 4 months ago by GerdP

corrected patch (one quote was missing and paths caused trouble)

comment:7 Changed 4 months ago by GerdP

I just wondered about the spelling. The patch uses "Data Validator", in other sources I find "Data validator" and "OSM data validator". I think I prefer "OSM data validator" or just "validation".

comment:8 Changed 4 months ago by Hb---

Please use "Data validator".

The longer term "OSM data validator" may lead to the conclusion that the rules are from OSM.

comment:9 Changed 4 months ago by skyper

"JOSM data validator" would work, if we want a longer form.

comment:10 Changed 4 months ago by Don-vip

Milestone: 21.1021.11

Milestone renamed

comment:11 Changed 3 months ago by GerdP

In 18318/josm:

see #20378:No Warnings on Crossings

  • improve I18n string (patch by Hb---, modified)

comment:12 Changed 3 months ago by GerdP

Resolution: fixed
Status: assignedclosed

I think this ticket is just about the text in the dialog, so it can be closed.

comment:13 Changed 6 weeks ago by Don-vip

Milestone: 21.1121.12

Milestone renamed

Modify Ticket

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