Opened 11 years ago
Last modified 4 years ago
#10226 new enhancement
Verify multiple equal values
Reported by: | naoliv | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | Core validator | Version: | |
Keywords: | mapcss multiple values repetition | Cc: | bastiK, simon04 |
Description
Could JOSM verify for multiple equal values?
For example, ref
allows multiple values but if we have ref=A1;A1
we do not see any warning.
Since ref
allows multiple values, it should warn only when we have two or more equal values in the set.
Could also be expanded for other keys that allow multiple values.
Attachments (0)
Change History (9)
comment:2 by , 11 years ago
In the first moment I was a little bit uncertain, if we could define a general rule, like
- split() values at ';'
- trim() the values (values are separated by ";" or by"; " like
tracktype=grade1;grade2
andtracktype=grade1; grade2
). Perhaps an other possible validator check, because the space after ";" is not correct (as far as I understand the wiki) and should be detected by the normal "value starts with space" rule. - find duplicates
There are several cases where we have more than one value for a key:
- a list of values (which are not mutually exclusive) like
cuisine=kebab;pizza
. In this case it makes never sense for multiple equal values. - mutually exclusive values make never sense (like
lit=yes;no
). This could be an other validation test, if we could identify mutual exclusive values :-). - an array of values like
*:lanes=*
. In this case equal values make sense, but we have a different delimiter '|'. Also in this case there could be values separated by ';' liketurn:lanes:forward=left;through|through|through;right
.
For the 3rd case I can construct a false positive:
In turn:lanes:forward=left;through|left;through|left;through|right
"through|left" appears twice!
We could solve this problem by independently checking each '|' separated value:
for (String arrayValue : value.split("|")) { HashSet<String> values = new HashSet<>(); for (String value : arrayValue.split(";") { value = value.trim(); if (values.contains(value) { warn("multiple equal value " + value); } else { values.add(value); } { }
Are there more than these 3 scenarios?
Is there any other scenario where we could have a false positive?
follow-up: 5 comment:4 by , 10 years ago
But oneway=* is a bad example, because multiple values makes no sense on this key. Nevertheless you will find several values like "yes;no", "no;yes", -1;yes" or "yes;-1". I try to correct them the the last months, but they pop up all the time.
I analysed some cases:
Way 315704369: Exists in the first version, but I don't think the mapper has really added oneway=yes;-1 (created_by=iD 1.6.2)
Is it possible, that iD deletes the old ways (and their history) when you merge two ways?
Is it possible, that iD merges values without warning, when you merge ways?
Way 256185821: Exists in the 3rd version. It looks like the mapper merge a fragmented roundabout into a single closed way (number of nodes changes from 4 to 21 and junction=roundabout is added. But also oneway=yes changes to oneway=yes;-1 (iD 1.6.2)
I thing again, that iD automatically merges the values.
BTW: does it make sense to have junction=roundabout and oneway=* on the same way? Or should JOSM warn about such a situation?
Similar: way 67550887 (iD 1.6.2)
Way 310599559: 1st version directly with oneway=yes;no (JOSM/1.5 (7588 es))
It's the result from a split of way 78560464, which also had oneway=yes;no, but was changed into oneway=yes.
The original way 78560464 is again the result of a merge where oneway=yes changes to oneway=yes;no (iD 1.5.4)
The last change, which produces the way 310599559 is a manual revert of the merge, where oneway=yes;no is left on the new created way.
Again id has merged the values on merging ways.
But JOSM did not warn warn about oneway=yes;no. The validator only creats an info, which is normally not shown on upload:
Value 'yes;no' for key 'oneway' not in presets. - Presets do not contain property value (1)
Conclusion:
- iD should stop merging values (without warning the user)!
- JOSM should warn about multiple values on oneway (or more general on keys which don't allow multiple values)
- JOSM may warn about oneway=* together with junction=roundabout
follow-up: 6 comment:5 by , 10 years ago
Replying to mdk:
- iD should stop merging values (without warning the user)!
The reason I *really* dislike iD is because of things like this:
We're not going to add an intrusive warning to iD for this case. Warnings may be well intentioned, but they run counter to iD's goal of encouraging new users to contribute to the map because they make them feel insecure, even when their edits are perfectly legitimate. "I might break something, I better not touch this map" -- and they walk away. Not to mention they are an annoyance for experienced mappers, who think "of course I want to do what I said I wanted to do", and click through without reading. In the majority of cases, merging with ';' is the correct behavior; our plan for covering the remaining cases is outlined in #908 (adding an unobtrusive warning).
https://github.com/openstreetmap/iD/issues/1480
There are other examples about this too.
follow-up: 7 comment:6 by , 10 years ago
Replying to naoliv:
Replying to mdk:
- iD should stop merging values (without warning the user)!
The reason I *really* dislike iD is because of things like this:
We're not going to add an intrusive warning to iD for this case. Warnings may be well intentioned, but they run counter to iD's goal of encouraging new users to contribute to the map because they make them feel insecure, even when their edits are perfectly legitimate. "I might break something, I better not touch this map" -- and they walk away. Not to mention they are an annoyance for experienced mappers, who think "of course I want to do what I said I wanted to do", and click through without reading. In the majority of cases, merging with ';' is the correct behavior; our plan for covering the remaining cases is outlined in #908 (adding an unobtrusive warning).
https://github.com/openstreetmap/iD/issues/1480
There are other examples about this too.
Hope, they do not destroy relations this way.
Never understood, why you need "merge ways" in iD. It is simply no task for new contributes.
follow-up: 8 comment:7 by , 10 years ago
Replying to skyper:
Hope, they do not destroy relations this way.
comment:8 by , 10 years ago
comment:9 by , 4 years ago
Keywords: | multiple values repetition added |
---|
With #15751 in mind, it might be useful to have a separate test for oneway value. I still like the idea of this ticket, though, and maybe we can fix it within the next seven years.
it could be interesting for:
source
,source:*
,attribution
,note
,description
name
,*_name
ref
,*_ref
destination
,addr:housenumber
,voltage
,traffic_sign
,cuisine
Could we do that using MapCSS? We already have source:trunk/resources/data/validator/multiple.mapcss