iD should not combine ways with different tags #2358
Comments
|
What happens when one way has tags and the other does not? Like, two sections of road, one with a name and one with no name. It may be perfectly legit to combine them because it's really all one road, but you don't want to have to type in the name on the other piece before they will combine. Another example is when it's two sections of the same road with different speed limits. You really don't want to combine those, but the speed limit tag is often off the bottom of the screen so the two ways look the same unless you bother to scroll down. |
|
+1 for this issue. I think that especially in iD (which is targeted mainly at users with less mapping experience), it is very important to warn the user when they make erroneous edits. |
|
+1 - I spent 3 hours undoing many merges of a road with different relation members, including the final merge which ended up with name=East McBee;WestMcBee . I just rechecked the ID behavior today, and there is no warning when joining road segments that are members of different relations. |
|
Perhaps, if nothing else can be done right now, the entire capacity to merge roads should be disabled. After all, is it EVER truly necessary to merge roads? The only time it seems to make any noticeable difference on the finished map is that the same road name may appear twice in rapid succession, but that can be corrected by simply deleting the name from one of the smaller sections. |
|
So, I will pick up from this comment, as I was locked out of the previous thread after being called a troll. A troll... right. I might as well abandon OSM after that. I guess map maintainers are not allowed to point out issues, perhaps they should just shut up and keep fixing what gets broken instead of doing more useful work. If I really believed iD was bad, I wouldn't be here, I would just tell people to use another editor. iD is awesome (I've said that several times) and it has only one major flaw (this one). Back to rationality, @tmcw said that there were two important points:
Makes this issue even more important. But mappers should be aware of what is happening to relations. If the mapper deletes something by accident, and does not know relations, he/she might try to remap what has been deleted, and the new object will then no longer belong to that relation. Most new users (and there is a lot of them these days) start mapping without contacting the community or reading the documentation, so knowing relations cannot be expected.
When deleting any member(s) of any relation(s) in JOSM (sorry, again), the user is presented with a window showing the parent relations and asks for confirmation. This is complemented by a validator that checks if the user is submitting broken relations when committing changes. A validator is very complex and I understand that iD developers may not want to go that far. Trying to update relations cleverly will require code on par with the complexity of a validator. But similar to what has been done for the delete operation, iD could simply disable the merge button when not all selected ways have the same set of parents, and tell the user to check their membership. This would become a "chore" only when it should be a chore. JOSM handles tag merging by exposing the user to the raw tags assigned to the ways, and this is clearly undesirable on iD, which prefers to interpret these tags in a friendly manner. What is expected if a user combines, in a single operation, a street with maxspeed set, another with maxspeed set to a different value, another without maxspeed, a waterway and a power line? The mapper needs to know a few things clearly before proceeding:
In this example, the user would need to know that combining the three different presets of selected ways result in [whatever preset], that [maxspeed's friendly name] is missing in three of them, and that maxspeed will be set to [show final value]. There's no need to let the user modify these properties at this stage, this could be done after accepting the operation. One suggestion: if there is any tag difference, display a small alert flag at the corner of the merge button, and if the user does click the button, then present a modal listing these three pieces of information, asking the user to accept or reject. Users will understand the flag implies a "complex modal", and if they're bothered by that, they may choose to avoid it when the icon is present. Well, these are my honest thoughts. I hope I won't be called a troll anymore. |
Thank you for the kind words..
What if a user wants to combine ways that have the tags |
|
This ticket should split.
The first case is handled in JOSM well, in that it shows you the proposed merge of values. Often these need editing to make sense (tiger:cfcc=A45;A46 is generally not the right answer). The second could just flat out be prohibited in a beginner editor. A person using iD for certain editing tasks does not have a sharp enough knife at hand, and perhaps gently should be directed to either rethink what they're doing, or seek assistance to make the edit via a map note or mailing list post. |
|
@bhousel Generically, I think a warning should be issued, otherwise combining these two ways will result in a tag In my understanding, the CFCC tag is supposed to help people manually classify roads in the US. I'm not sure this is still valuable because I see tertiaries on the map and this article does not map any CFCC value to tertiaries. It could still be valuable for specific data consumers (I don't know any), in which case a warning should be issued and the mapper would prefer not to combine the ways. Combining them would lose the distinction of CFCC class in different parts of the resulting way. If no data consumer is interested in this information, it's not entirely impossible (though that should be discussed with the community) that this information can indeed be lost. Possibly functional classification is now more widely adopted by the American community, it makes sense based on what I've been researching. |
|
@brycenesbitt Is it fair to block users from editing relations? Some of the nicest things in OSM (road/bus/cycle/hiking/boat routes, polygons with holes, administrative boundaries, etc.) are relations. Users just need to know that they exist and know when they're potentially messing up something - that's probably enough to get (a vast majority of) them to search for the proper way to handle them. |
|
@ftrebien it's common to edit items that are part of a relation. It is rare to have a legitimate need to combine relations. |
|
So, yesterday I've uploaded the suburb limits of my hometown. Today, an iD user broke a boundary (see here if it's already been fixed), doing precisely what I have described: unaware of the boundary relation, he simply combined two ways, with one of them (a tiny one) being part of the boundary. He had no particular reason to combine the ways other than personal preference. My hometown has about three active users, it's not an intensively edited area such as Berlin, London or Paris. Maybe that's why Berlin's suburb boundaries use extra boundary lines parallel to boundary streets. |
|
Now imagine what will happen if I move on to map all 260 bus routes here, which, different from suburbs, strictly require the use of streets as members. |
|
Oh, and I see that iD already prevents users from combining ways that belong to relation=restriction (turn restrictions)... |
|
It should be strictly rejected by community an buggy edition software breaking or decreasing the quality of the database of way such relevant. |
|
As of 8 months ago, it silently would combine street segments with different transport relations... http://www.openstreetmap.org/changeset/26199248 . It should hopefully be straightforward to extend the rules that prevent combining ways with different turn restriction relations. |
|
Before combining ways, a list of selected ways is presented on the left. For few ways (the most common scenario), there is usually a lot of empty space below the list. How about using that space to display the differences in tags/fields/preset across the selected ways? This wouldn't block the user from proceeding, and would make him/her aware of the consequences (it is only 100% ok to combine them if no differences are shown), and it would also allow him/her to take editing shortcuts (if the user knows that it's ok to propagate tags, or plans to fix them afterwards). |
I agree.. It turns out that #908 already deals with merged tags, and I wish I had noticed this sooner before the discussion about the tag merges. It's an open issue, so definitely something we want to improve.
I like this suggestion a lot. The big issue I have is (in case it wasn't clear from my A few months ago when I implemented the conflict resolution feature, this bothered me a lot as well. I eventually decided that during conflict resolution it is ok to tell the user about the conflicting tags because I found a way to let them choose to keep the remote version of the tag if they don't know what the conflict is about (and indeed we make that the default choice). But here we are discussing the geometry merging operation, which is different... I'm leaning towards warning the user if it is a tag settable by a UI field ( I also think it is reasonable for iD to have a special "warning" style for UI fields that have a weird value in them. One common way I have seen this implemented in other systems would be if we support a validation regex in the These are my thoughts about the tag merging, and I'll discuss relation merging separately. |
So... It's no secret that iD sucks at relations. This is a hard problem for reasons that @tmcw explained really well on OSM-talk. There are a handful of different kinds of relations defined by the To be honest, I have been putting off dealing with any of the relation issues until we implement something like #659 - because really we should be able to show the user visually what relations a feature belongs to. For me anyway, that's a prerequisite to even asking the user to make a decision about relations. I don't know what the UI for this would look like. Maybe a small relation icon with a numeric badge, and if you press the icon it opens a panel with a legend showing you what relations pass through the viewport. Not the multipolygons or the turn restrictions, or associatedStreets, but yes to the administrative boundaries, coastlines, cycle routes, and transit stuff, site relations, etc. With different color shadows under the features.. In the mean time, yes it's crap that bus routes are getting broken. I'll try experimenting with locking down some of the operations (splits and merges) to prevent this from happening, and see if it makes editing urban areas and major roads too cumbersome. |
|
Thank you, @bhousel . Splitting ways is always ok, no need to check anything. Merging them is ok too if all selected ways belong to the exact same set of relations. You can always merge back what someone just split as long as they don't change memberships. This would probably alleviate some (maybe many) of the cases in which locking would bother the mapper. (I'm really trying to suggest the best compromise I can imagine without adding too much overhead/complexity.) |
This is, sadly, not true. If you have the ways A and B and node 1 in with a turn restriction relation with from=A, via=1, to=B, merging A and B changes the meaning. There is a similar case with via ways. |
|
Copying a bit from the mailing list, responding to @tmcw saying relationships are invisible. Relationships are invisible only if your editor leaves them invisible. Relations are not only possible to visualize, they're interesting, Click on Main Street and see the 12 bus lines that use Main street? Interesting. Click on a line and learn it forms the USA/Canadian Border, 8,891 km long, consisting of 5000 odd line segments? Interesting and instructive. A series of iD plugins for visualizing specific types of relations would rock. And of course in iD style Relations are invisible only in editors that leave them in the shadows. An editor that ignores or tries to hide a thing is unlikely to be the best way to edit (or preserve) the invisible thing. It's a form of "fake simplicity": making a given edit seem to be simpler than it really can be. If nothing else appears, this IS a case for a modal: You are about to merge or split objects which are part of a grouping called a relation. This can have unintended consequences. It is recommended you find a different way to accomplish this task, and if needed contact another mapper via a note or mailing list The ID social feature can help you find a mapper active in your area This message should be edit time, NOT save time. See separate ticket about making SAVE more clear ( #1735 ) |
|
@pnorman You're right, I'm too used to thinking that turn restrictions are already handled automagically. This unfortunately makes the algorithm more complex:
This will will block editing of some no_entry and no_exit restrictions, which may have multiple from and to members. They are very very rare though. |
|
Hm there's one more thing: checking membership must also check that the role is the same across relations. Because the way can be included multiple times in a relation with different roles, each with its own multiplicity, this adds yet another layer of complexity. :-/ There must be a better, yet simple approach (will think). |
|
This problem has bitten me more than once, +1 on improving this ASAP. @ftrebien seems to have a good understanding of this issue, please consider his suggestions when fixing this. |
|
Thank you @afmenez . Fortunately, I'm not alone. I'm assuming it is not iD's responsibility to fix broken relations automatically, only to ensure that consistent relations stay consistent, so situations like "same way twice on a multipolygon" or "discontinuous routes" would not be covered (the mapper should fix them, if they understand the problem). If editors ensure relations stay consistent, such cases (which are rare) can only become rarer. I've drawn up a few test cases + corner cases and the following Pythonesque pseudocode (with lax syntax to aid understaing) seems to work in all situations as long as relations are not in a broken state before merging (still in brainstorm phase, please point out mistakes): # combines route role and way direction into an "actual role" that must be maintained
# does not change roles that are not forward/backward, which also must be maintained
def actualRole(way, role):
if way.direction is undefined or way.direction == role:
return role
if way.direction == "backward":
return "forward"
else:
return "backward"
# assumes ways are connected at endnodes (merge button is available to user)
def mergeLock(ways):
relations = getParentRelations(ways)
sortedWays = sortByConnectivity(selectedWays)
# directions: for ensuring "actual role" is maintained
# connections: for checking restrictions and destination signs
way = sortedWays.first
for i in range(1, sortedWays.len-1):
nextWay = sortedWays[i]
if (way.nodes.last == nextWay.nodes.first or
way.nodes.last == nextWay.nodes.last):
way.direction = "forward"
connections.add(way.nodes.last)
else:
way.direction = "backward"
connections.add(way.nodes.first)
way = nextWay
previousWay = sortedWays[sortedWays.len-2]
if (way.nodes.first == previousWay.nodes.last or
way.nodes.first == previousWay.nodes.first):
way.direction = "forward"
else:
way.direction = "backward"
nextWay = sortedWays.first
if (way.nodes.last == nextWay.nodes.first or
way.nodes.last == nextWay.nodes.last):
connections.add(way.nodes.last)
if (way.nodes.first == nextWay.nodes.first or
way.nodes.first == nextWay.nodes.last):
connections.add(way.nodes.first)
for relation in relations:
# via node: must not be a connection between selected ways
# via ways: must span the entire selection, or none of it
if relation.type == "restriction":
vias = set([])
for member in relation.members:
if member.role == "via":
if member.primitive == "way":
vias.add(member.way)
if member.primitive == "node":
if member.node in connections:
return "Merging would break restriction relation from " + \
relation.members['from'].name + " to " + \
relation.members['to'].name
if vias.len > 0 and not vias.equalsSet(ways):
return "Merging would break restriction relation passing through selection"
# nodes: must not be a connection between selected ways if
# role is intersection, to, or from
# ways: must span the entire selection if role is to or from
elif relation.type == "destination_sign":
tos = set([])
froms = set([])
for member in relation.members:
if member.primitive == "way":
if member.role == "to":
tos.add(member.way)
if member.role == "from":
froms.add(member.way)
if member.primitive == "node":
if member.role != "sign" and member.node in connections:
return "Merging would break destination sign relation to " + \
relation.members['to'].name
if (tos.len > 0 and not tos.equalsSet(ways)) or
(froms.len > 0 and not froms.equalsSet(ways)):
return "Merging would break destination sign relation to" + \
relation.members['to'].name
# selection must come in sequences of constant "actual" role
elif relation.type == "route":
i = 0
while i < relation.members.len:
member = relation.members[i]
j = i + 1
if member.way in ways:
groupWays = set([member.way])
groupRole = actualRole(member.way, member.role)
while j < relation.members.len:
otherMember = relation.members[j]
if otherMember.way in ways and actualRole(otherMember) == groupRole:
groupWays.add(otherMember.way)
j = i + 1
else:
break
if not groupWays.equalsSet(ways):
return "Merging would break route relation continuity in" + \
relation.name
i = j
# selection must be entirely included with constant role
else:
roles = set([])
for way in ways:
if way not in relation.members:
return "Merging would break the " + relation.type + " relation " + \
relation.name
roles.add(member.role)
if roles.len > 1:
return "Merging would break the " + relation.type + " relation " + \
relation.name
return None |
|
Alternately, iD could more simply decline to merge/split ways that are part of relations, I'll bet that 90% of the users would do without the split or merge they propose, if |
|
Above you talk about "just silently performing the semicolon merge if it is a tag that is hidden anyway and only available in the raw tag editor". You should also make sure any tags that have *:lanes in them should also be warned about, even if they are hidden. Just recently on [talk-us], it went nuclear because a user went crazy because he though somebody went intentionally vandalizing all of his *:lane work on a major highway in Washington state. What he failed to realize was that the user did his edits with iD and wasn't warned that when he was merging some of the ways, it damaged all the *:lanes that where there. When this was noticed by me that it was iD to blame, not the user totally since iD didn't warn him, everything calmed down some. |
Which segments of WA 500 were damaged by iD exactly? I did take a look earlier today but didn't see anything that looked like a tag merge with semicolons. For example, if someone merges conflicting lane tags, you would end up with something like: |
|
@bhousel It messed up the following tags: All because several segments were merged together without letting the user of iD knowing they are doing some major damage. |
|
The "different relation membership" is already covered by #1512 (and note that iD already has some relation-damage prevention built in), so I'm retitling this to be about tags only. |
jfirebaugh
changed the title
Way to reproduce (example):
I expect that iD can not combine these ways, or give the possibility for choosing one of the values of tracktype (like in JOSM).
Another example:
I expect the same as stated above: iD should not give the possibility to combine these ways, simply because they are different.
For some time I had much work to restore ways and relations that were combined with iD and now had double values in keys. Restoring the ways was relatively easy, reverting the relations was much more difficult because it is not easy to see to which way the relation belonged to. I think I even couldn’t restore all the relations, so good information was destroyed because of the incorrect edits with iD!
I think that the best solution for this problem is to make the + (combine) icon not available when ways have different tags or are different member of a relation (select two not connected ways to see what I mean with the + icon: the icon is there, but not available). If the combine icon is hidden and newbies (or other iD users) see the message that it isn't possible to combine the ways, I think it doesn't matter them much. In any case it is better than damaging the data! Besides that, because of the aimed simplicity of iD, I think it isn't very important that in some situations ways couldn't be combined. Indeed this makes iD more simple!
The text was updated successfully, but these errors were encountered: