Modify

Opened 4 months ago

Closed 7 weeks ago

Last modified 4 weeks ago

#18670 closed defect (fixed)

Unglue ways: False positives about affected relations

Reported by: skyper Owned by: GerdP
Priority: normal Milestone: 20.05
Component: Core Version:
Keywords: template_report unglue way relation Cc:

Description

What steps will reproduce the problem?

  1. Have a way (A) with three nodes and member of a relation and another way (B) connected at the middle node without membership, see attached file
  2. select middle node and way A
  3. unglue ways

What is the expected result?

No info warning

What happens instead?

Info warning about affected relation.

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

Please, do not show this warning if the selected node is a middle node of the selected way and the other way is not member of the relation.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-02-02 18:47:18 +0100 (Sun, 02 Feb 2020)
Revision:15810
Build-Date:2020-02-03 02:30:51
URL:https://josm.openstreetmap.de/svn/trunk

Attachments (7)

josm_unglue_relation.osm (14.5 KB) - added by skyper 4 months ago.
example file
18670.patch (817 bytes) - added by GerdP 2 months ago.
18670.2.patch (6.2 KB) - added by GerdP 2 months ago.
suppress warning notification for parent relation when unglued node is not first or last node of modified way
18670-sample.osm (1.5 KB) - added by GerdP 2 months ago.
18670.3.patch (8.3 KB) - added by GerdP 8 weeks ago.
18670.4.patch (10.1 KB) - added by GerdP 8 weeks ago.
suppress warning notification for some parent relation types ("restriction", "connectivity", "destination_sign") when they are not affected
18670.5.patch (9.9 KB) - added by GerdP 8 weeks ago.
also handles the case that a via way is unglued

Download all attachments as: .zip

Change History (25)

Changed 4 months ago by skyper

Attachment: josm_unglue_relation.osm added

example file

comment:1 Changed 2 months ago by GerdP

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

Changed 2 months ago by GerdP

Attachment: 18670.patch added

comment:2 Changed 2 months ago by GerdP

WIP : There are more cases to fix in this action.

Changed 2 months ago by GerdP

Attachment: 18670.2.patch added

suppress warning notification for parent relation when unglued node is not first or last node of modified way

comment:3 Changed 2 months ago by skyper

I would say if neither the selected node nor the selected way is part of the relation suppress any warning even for end nodes.

For the node problem see #6836.

Last edited 2 months ago by skyper (previous) (diff)

Changed 2 months ago by GerdP

Attachment: 18670-sample.osm added

comment:4 Changed 2 months ago by GerdP

Do you think about the case 18670-sample.osm when the node at the western end of the from way is selected for unglue?

comment:5 in reply to:  4 Changed 2 months ago by skyper

Replying to GerdP:

Do you think about the case 18670-sample.osm when the node at the western end of the from way is selected for unglue?

I was thinking about the situation splitting the way with three nodes of josm_unglue_relation.osm at the middle node and then unglueing but this is working already.

Your example is valid, though.

comment:6 Changed 2 months ago by GerdP

I wonder how we should detect that a change of the way doesn't effect the relation. It requires knowledge about the relation type (=lots of code) to decide that a relation is not affected by the change.
Example: The relation might be a type=multipolygon. In that case the notification makes sense if the from way was selected, else see #19042.
Also, when I unglue a way from a via node and the way is not member of the turn restriction it might still mean that the turn restriction is obsolete (as in the attached sampe). The current tested version r16239 doesn't produce a warning for this case, it just asks if the via role should be copied (unless we apply the patch for #6836).

I'd say: Better safe than sorry.
A improvement would be to change the text in the notification:

        final String msg1 = trn("Unglueing affected {0} relation: {1}", "Unglueing affected {0} relations: {1}",
                size, size, Utils.joinAsHtmlUnorderedList(affectedRelations));

I would change that to

        final String msg1 = trn("Unglueing possibly affected {0} relation: {1}", "Unglueing possibly affected {0} relations: {1}",
                size, size, Utils.joinAsHtmlUnorderedList(affectedRelations));

and try to show the notification also for the parent relations of the involved node(s).

comment:7 Changed 2 months ago by skyper

+1 for adjusting the warning (for now).

Something was changed as josm_unglue_relation.osm works as expected (r16243).

I do not know any relation which is changed/damaged if the middle node of a member way is deleted unless that node is also member of the relation.

There is a difference between modifying/damaging a relation and making a turn-restriction obsolete. For the later a validator warning could be enough and no real damage is possible if there is no option to adjust the membership.

Relations with via members always need a special treatment as you need to look at the way and its end nodes and only warn if both, the way and its child end node, are members. Split-way does handle it but unglue not.

Last edited 8 weeks ago by skyper (previous) (diff)

comment:8 in reply to:  7 ; Changed 8 weeks ago by GerdP

Replying to skyper:

+1 for adjusting the warning (for now).

Something was changed as josm_unglue_relation.osm works as expected (r16243).

No, I still get the notification.

I do not know any relation which is changed/damaged if the middle node of a member way is deleted unless that node is also member of the relation.

I also don't know any, but I only know MP and those relations which affect routing well as they are handled by mkgmap.

There is a difference between modifying/damaging a relation and making a turn-restriction obsolete. For the later a validator warning could be enough and no real damage is possible if there is no option to adjust the membership.

OK

Relations with via members always need a special treatment as you need to look at the way and its end nodes and only warn if both, the way and its child end node, are members. Split-way does handle it but unglue not.

Changed 8 weeks ago by GerdP

Attachment: 18670.3.patch added

comment:9 Changed 8 weeks ago by GerdP

18670.3.patch improves the way selection if only a node is selected for unglue and changes the message as suggested.
It still produces a few false positives reg. the notification.

comment:10 in reply to:  8 Changed 8 weeks ago by skyper

Replying to GerdP:

Replying to skyper:

Something was changed as josm_unglue_relation.osm works as expected (r16243).

No, I still get the notification.

Strange, I do not get it neither with r16251 nor r15937.

Last edited 8 weeks ago by skyper (previous) (diff)

comment:11 Changed 8 weeks ago by GerdP

Make sure that you select the longer way and the node

comment:12 Changed 8 weeks ago by skyper

Sorry, you are right.

comment:13 Changed 8 weeks ago by GerdP

Relations with via members always need a special treatment as you need to look at the way and its end nodes and only warn if >both, the way and its child end node, are members. Split-way does handle it but unglue not.

Working on this. Struggling with the case that the relation has via way(s), not nodes.

comment:14 Changed 8 weeks ago by GerdP

BTW: split-way doesn't seem to handle via ways. When I split one the relation is not updated.

Changed 8 weeks ago by GerdP

Attachment: 18670.4.patch added

suppress warning notification for some parent relation types ("restriction", "connectivity", "destination_sign") when they are not affected

comment:15 Changed 8 weeks ago by GerdP

still doesn't work when node between two via ways is unglued. Just found that case in #9008

Changed 8 weeks ago by GerdP

Attachment: 18670.5.patch added

also handles the case that a via way is unglued

comment:16 Changed 7 weeks ago by GerdP

Resolution: fixed
Status: assignedclosed

In 16300/josm:

fix #18670, #19042: Unglue ways: False positives about affected relations

  • avoid to warn about possibly broken turn restrictions when we can make sure they are not broken
  • when no way is selected, try to find one that produces fewer updates

TODO: handle dataset without download areas

comment:17 Changed 7 weeks ago by GerdP

In 16303/josm:

see #18670: fix error introduced with r16300
Ungluing an unconnected tagged node (e.g. natural=tree) caused exception instead of "not glued to anything" message

comment:18 Changed 4 weeks ago by Klumbumbus

Milestone: 20.0420.05

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.