Modify

Opened 8 years ago

Closed 8 years ago

#7061 closed enhancement (fixed)

[PATCH] Warn about head-to-head or back-to-back natural=coastline ways

Reported by: skela Owned by: Daeron
Priority: normal Milestone:
Component: Core validator Version: tested
Keywords: coastline Cc:

Description

An OSM newbie edited the coastline last weekend. He had deleted existing natural=coastline ways and replaced them with his own edits, and he forgot to connect the nodes.

I fixed that by extracting the natural=coastline ways from the finland.osm.pbf with Osmosis, loading it into JOSM and addressing the validator complaints.

All well and good, but the flooding did not cease today. I was puzzled. JOSM did not complain anything about this. Even the latest JOSM tested as of today does not complain anything about this.

Finally, it occurred to me that the new coastline sections were the wrong way around (land to the right, instead of land to the left).

Could JOSM Validator please issue a warning when it notices that a natural=coastline way is connected to a natural=coastline way that runs in the opposite direction?

Attachments (1)

validator-coastline.patch (1.4 KB) - added by Daeron 8 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 8 years ago by skela

It looks like the UNORDERED_COASTLINE or REVERSED_COASTLINE check in josm/core/src/org/openstreetmap/josm/data/validation/tests/Coastlines.java is supposed to implement exactly this check. It was imported from the OSM repository in r3669.

I will try to see what is going on here, and what exactly these checks are trying to achieve.

comment:2 Changed 8 years ago by skela

The UNORDERED_COASTLINE seems to check for exactly this. I can get it to complain on a toy data file, but not on the real example of broken coastline. I will try to reduce the test case.

I do not understand yet what REVERSED_COASTLINE is checking. I do not remember it ever reporting anything. The message "Reversed coastline: land not on left side" is reported by WronglyOrderedWays.java, not by Coastlines.java.

comment:3 Changed 8 years ago by skela

I got it: UNORDERED_COASTLINE can complain if one way is head-to-head or back-to-back with its neighbours. If there are multiple ways, it remains silent. I tested by downloading some coastlines that I fixed in http://www.openstreetmap.org/browse/changeset/9822569 and by reversing two adjacent lines (→→←): Nothing. Reversing exactly one line (→←←) issued an error.

comment:4 Changed 8 years ago by Daeron

Owner: changed from team to Daeron
Status: newassigned

Changed 8 years ago by Daeron

Attachment: validator-coastline.patch added

comment:5 Changed 8 years ago by Daeron

Summary: Warn about head-to-head or back-to-back natural=coastline ways[PATCH] Warn about head-to-head or back-to-back natural=coastline ways

comment:6 Changed 8 years ago by Daeron

Attached a patch that should make validator to report also multiple consecutive reversed coast line ways as UNORDERED_COASTLINE. In this case only the nodes that connect two coastline ways either head to head, or back to back are highlighted, but not the way(s).

This however adds a bit of an unnecessary noise when there is just one reversed way, reporting also the adjacent ways as unordered, but that's technically correct too (and they are not highlighted, only the end nodes facing the reversed way are).

comment:7 in reply to:  6 Changed 8 years ago by skela

Replying to Daeron:

Attached a patch that should make validator to report also multiple consecutive reversed coast line ways as UNORDERED_COASTLINE. In this case only the nodes that connect two coastline ways either head to head, or back to back are highlighted, but not the way(s).

Thanks, this works.

This however adds a bit of an unnecessary noise when there is just one reversed way, reporting also the adjacent ways as unordered, but that's technically correct too (and they are not highlighted, only the end nodes facing the reversed way are).

I do not mind the extra noise. Compilers can also emit lots of errors for a single missing or extra character. The main thing is that the errors in my sample file are now detected properly. Thank you for the quick turnaround!

comment:8 Changed 8 years ago by stoecker

Resolution: fixed
Status: assignedclosed

In [4610/josm]:

fix #7061 - patch by Daeron - fix costline validation

Modify Ticket

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