#8519 closed enhancement (fixed)
Validator error for keys suffixed with :lanes
Reported by: | imagic | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | 14.01 |
Component: | Core validator | Version: | |
Keywords: | Cc: |
Description
The validator should show an error in the following case:
- count the number of the character | (vertical bar) in the value of keys which end with any of the following: :lanes, lanes:forward, lanes:backward, lanes:both_ways . Consider the value for each suffix separately.
- If for any suffix the number of the character | is greater than zero, all keys with the same suffix must have the same number of | . Otherwise show an error: In case of keys suffixed with :lanes show "Number of lane dependent values inconsistent". For keys with different suffixes show "Number of lane dependent values inconsistent in XXX direction" whereas XXX should be the direction suffix like forward, backward or both_ways.
Examples:
turn:lanes=left|right + change:lanes=only_left|not_right|yes
Error: Number of lane dependent values inconsistent
width:lanes:forward=1|2|3 + psv:lanes:forward=no|designated
Error: Number of lane dependent values inconsistent in forward direction
change:lanes:forward=yes|no + turn:lanes:backward=left|right|left
Ok - no error (different suffixes)
turn:lanes:forward=left|right + change:lanes:forward=yes|no|yes + width:backward=1|2|3
Error: Number of lane dependent values inconsistent in forward direction
Attachments (0)
Change History (29)
comment:1 by , 12 years ago
follow-up: 3 comment:2 by , 11 years ago
This would be great.
And all the found values should also be compared to the value of "lanes" and "lanes:forward" and "lanes:backward", whether they all logically match.
E.g. lanes=3 + lanes:forward=1 and lanes:backward=1 is an error. Or lanes:forward=1 + turn:lanes:forward=yes|no is an error.
follow-up: 4 comment:3 by , 11 years ago
Replying to aceman:
And all the found values should also be compared to the value of "lanes" and "lanes:forward" and "lanes:backward", whether they all logically match.
This is dangerous as the lanes key only count lanes for motorized traffic while all keys with the :lanes extension specify lane-dependent values for all kind of lanes, e.g. also bicycle lanes.
Or lanes:forward=1 + turn:lanes:forward=yes|no is an error.
This is not an error! There might be a lane that is not for motorized traffic!
follow-up: 5 comment:4 by , 11 years ago
Replying to imagic:
Replying to aceman:
And all the found values should also be compared to the value of "lanes" and "lanes:forward" and "lanes:backward", whether they all logically match.
This is dangerous as the lanes key only count lanes for motorized traffic while all keys with the :lanes extension specify lane-dependent values for all kind of lanes, e.g. also bicycle lanes.
Or lanes:forward=1 + turn:lanes:forward=yes|no is an error.
This is not an error! There might be a lane that is not for motorized traffic!
Yes, the value "yes" is an problem but not that you have more values than the value of lanes:*=
comment:5 by , 11 years ago
Replying to skyper:
Replying to imagic:
Replying to aceman:
And all the found values should also be compared to the value of "lanes" and "lanes:forward" and "lanes:backward", whether they all logically match.
This is dangerous as the lanes key only count lanes for motorized traffic while all keys with the :lanes extension specify lane-dependent values for all kind of lanes, e.g. also bicycle lanes.
Or lanes:forward=1 + turn:lanes:forward=yes|no is an error.
This is not an error! There might be a lane that is not for motorized traffic!
Yes, the value "yes" is an problem but not that you have more values than the value of lanes:*=
No. Once again: the key lanes count only lanes for motorised traffic. All keys with the :lanes suffix specify properties for all kind of lanes. If the road has (in forward direction) one lane for cars and one bicycle lane and the lane for cars is a left turn you would tag: lanes:forward=1 + turn:lanes:forward=left|none. Additional tags might be e.g. vehicle:lanes:forward=yes|no + bicycle:lanes=yes|designated. Please read the approved proposal of the :lanes-extension: http://wiki.openstreetmap.org/wiki/Proposed_features/lanes_General_Extension#The_issues_with_the_lanes_tag
follow-up: 7 comment:6 by , 11 years ago
If there are other than "motorized" lanes on the road then they should be tagged in some way. E.g. lanes:bicycle=1 or maybe determined from the counting of lanes in bicycle:lanes=*. Then, the validator can take that into account and as I said check if they LOGICALLY match. If the algorithm must take cyclelanes into account, so be it. I would still consider "lanes:forward=1 + turn:lanes:forward=left|none" for a warning if these are the only tags on the way. If there is a cycleway in there, it is not tagged and invisible to any routing/rendering.
Anyway, I hope you agree that if there are MORE lanes= than number of lanes mentioned in e.g. turn:lanes then it is a problem?
follow-up: 8 comment:7 by , 11 years ago
Replying to aceman:
If there are other than "motorized" lanes on the road then they should be tagged in some way. E.g. lanes:bicycle=1
or maybe determined from the counting of lanes in bicycle:lanes=*.
You can not restrict yourself to bicycle lanes (neither lanes:bicycle nor bicycle:lanes). See below for the reason - bicycle lanes are just an example. BTW: bicycle:lanes does NOT give you the number of bicycle lanes! Only if you count the designated values, but I wouldn't rely on that as the tagging it some kind of a mess here (official? designated? yes?)
Then, the validator can take that into account and as I said check if they LOGICALLY match. If the algorithm must take cyclelanes into account, so be it.
What's there to "logically match"? All xxx:lanes-keys must have the same number of lane-dependent values. This number is the total number of lanes. That's all the information that you get from those keys about the number of lanes. The only thing you could verify is that the value of the lanes-key must not be larger than the number of the lane-dependent values (in the respective direction).
I would still consider "lanes:forward=1 + turn:lanes:forward=left|none" for a warning if these are the only tags on the way. If there is a cycleway in there, it is not tagged and invisible to any routing/rendering.
No. The validator does not have enough information to give a warning. Bicycle lanes were just an example. There could be other kinds of lanes that the validator doesn't know of and a warning would scare off mappers and possible prevent them from applying completely correct tags.
Anyway, I hope you agree that if there are MORE lanes= than number of lanes mentioned in e.g. turn:lanes then it is a problem?
Yes, that's clearly an error.
follow-up: 14 comment:8 by , 11 years ago
Replying to imagic:
You can not restrict yourself to bicycle lanes (neither lanes:bicycle nor bicycle:lanes). See below for the reason - bicycle lanes are just an example. BTW: bicycle:lanes does NOT give you the number of bicycle lanes! Only if you count the designated values, but I wouldn't rely on that as the tagging it some kind of a mess here (official? designated? yes?)
What's there to "logically match"? All xxx:lanes-keys must have the same number of lane-dependent values. This number is the total number of lanes. That's all the information that you get from those keys about the number of lanes. The only thing you could verify is that the value of the lanes-key must not be larger than the number of the lane-dependent values (in the respective direction).
lanes <= count_of(*:lanes) (for each *) (1st check proposed in the ticket)
lanes = lanes:forward + lanes:backward (2nd check proposed by me)
lanes + lanes:non-motorized = count_of(*:lanes) (for each *) (3rd check proposed by me)
I would still consider "lanes:forward=1 + turn:lanes:forward=left|none" for a warning if these are the only tags on the way. If there is a cycleway in there, it is not tagged and invisible to any routing/rendering.
No. The validator does not have enough information to give a warning. Bicycle lanes were just an example. There could be other kinds of lanes that the validator doesn't know of and a warning would scare off mappers and possible prevent them from applying completely correct tags.
What is the value of the turn:lanes (etc) having more lanes than are visible via 'lanes' ? Then you don't know which of the marked turn lanes belong to the motorized lanes and which do not.
I just say that for this tagging to have any value is to find out the number/order of non-motorized lanes from lanes:non-motorized and non-motorized:lanes. If there is no consensus yet on how to do that, then I agree, the check can't be done yet (the 3rd check of mine). But then the tagging is inadequate and needs to be sorted out in a proposal on wiki.
comment:9 by , 11 years ago
I have found that the "Lanes and Road attributes" map style (available from inside JOSM preferences or at http://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes) already does warn of some inconsistencies of lanes and also can draw e.g. turn:lanes. I have just found it and it seems a big help when focusing on tagging lanes on roads.
comment:11 by , 11 years ago
imagic: Thank you a lot for giving those 4 examples with expected result. Those have been converted 1:1 to unit tests source:trunk/test/unit/org/openstreetmap/josm/data/validation/tests/LanesTest.groovy
follow-up: 13 comment:12 by , 11 years ago
OK, so this seems to check the consistency of all *:lanes tags. Should I file a new ticket for the other 2 checks from my comment 8?
I don't know what the 'change' key is, but it is good the check also processes unknown keys. The key name actually isn't important, only the number of its values.
comment:13 by , 11 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Replying to aceman:
OK, so this seems to check the consistency of all *:lanes tags. Should I file a new ticket for the other 2 checks from my comment 8?
No, let's reopen this one. I would like to hear imagic's opinion on the other tests. The long discussion here doesn't make it easier to determine what to implement and what not.
I don't know what the 'change' key is, but it is good the check also processes unknown keys. The key name actually isn't important, only the number of its values.
To which change key are you referring to?
follow-up: 15 comment:14 by , 11 years ago
Replying to aceman:
lanes <= count_of(*:lanes) (for each *) (1st check proposed in the ticket)
This would make sense. You could also test for the suffixes :forward/:backward in this context, i.e.:
lanes <= count_of(*:lanes)
lanes:forward <= count_of(*:lanes:forward)
lanes:backward <= count_of(*:lanes:backward)
lanes = lanes:forward + lanes:backward (2nd check proposed by me)
Not a good idea, because there are some lanes, which are dedicated to both directions, i.e. center turning lanes. Have a look: http://www.fhwa.dot.gov/publications/research/safety/08042/images/image024.jpg
I proposed(!) the suffix :both_ways for those, but it is not really in use right now, therefore I would not test for it.
One could test for lanes >= lanes:forward + lanes:backward . I personally(!) would not do this, because I either tag (i) only lanes OR (ii) only lanes:forward and lanes:backward. I never tag lanes AND lanes:forward AND lanes:backward, but as this is just a personal preference this test would make sense.
lanes + lanes:non-motorized = count_of(*:lanes) (for each *) (3rd check proposed by me)
Very bad idea, because there is simply no tag like "lanes:non-motorized". If you want the number of all lanes, you have to count the values of any :lanes-key. It is a well known deficit of the lanes-key that there is no possibility to specify the count of all lanes.
Replying to simon04:
Replying to aceman:
I don't know what the 'change' key is
http://wiki.openstreetmap.org/wiki/Proposed_features/change
It is more or less defacto approved: http://taginfo.openstreetmap.org/search?q=change
follow-up: 19 comment:15 by , 11 years ago
Replying to imagic:
Replying to aceman:
lanes <= count_of(*:lanes) (for each *) (1st check proposed in the ticket)
This would make sense. You could also test for the suffixes :forward/:backward in this context, i.e.:
lanes <= count_of(*:lanes)
lanes:forward <= count_of(*:lanes:forward)
lanes:backward <= count_of(*:lanes:backward)
Thanks. Yes, as you say "lanes" may not exists on a way, so only do the check that is applicable (for lanes:forward or lanes:backward).
lanes = lanes:forward + lanes:backward (2nd check proposed by me)
Not a good idea, because there are some lanes, which are dedicated to both directions, i.e. center turning lanes. Have a look: http://www.fhwa.dot.gov/publications/research/safety/08042/images/image024.jpg
I proposed(!) the suffix :both_ways for those, but it is not really in use right now, therefore I would not test for it.
One could test for lanes >= lanes:forward + lanes:backward . I personally(!) would not do this, because I either tag (i) only lanes OR (ii) only lanes:forward and lanes:backward. I never tag lanes AND lanes:forward AND lanes:backward, but as this is just a personal preference this test would make sense.
I always tag "lanes" and "lanes:forward" and "lanes:backward" explicitly if lanes:forward != (lanes / 2) AND lanes:backward != (lanes / 2). But I think I have seen objects with "lanes" and only "lanes:forward" specified, so that the reader must infer the number for lanes:backward.
But the wiki says for now:
If the lanes on a two way road are not distributed evenly between the driving directions, the keys lanes:forward=* and lanes:backward=* can be used in addition to the lanes tag.
So it looks like "lanes" should be there, regardless of existence of "lanes:forward" or "lanes:backward" .
So the check could be modified like this:
IF "lanes" exists then:
- IF both "lanes:forward" AND "lanes:backward" exist, check lanes = lanes:forward + lanes:backward
- IF only "lanes:forward" OR only "lanes:backward" exists, check "lanes >= lanes:forward" OR "lanes >= lanes:backward" (the one that exists).
Otherwise, no warning, if you wish so. Even though I would personally issue a warning if there is lanes:forward OR lanes:backward and no "lanes" as the wiki says that is not to spec.
lanes + lanes:non-motorized = count_of(*:lanes) (for each *) (3rd check proposed by me)
Very bad idea, because there is simply no tag like "lanes:non-motorized". If you want the number of all lanes, you have to count the values of any :lanes-key. It is a well known deficit of the lanes-key that there is no possibility to specify the count of all lanes.
"lanes:non-motorized" was a shorthand for "lanes:foot"+"lanes:bicycle"+"lanes:horse"+etc. But of course if people regularly do not tag those then there is no check possible. I just said above I would expect a "lanes:bicycle" to be there as without it the information of the road is incomplete.
comment:17 by , 11 years ago
What is this?
void test6() {
lanes.check(TestUtils.createPrimitive("way lanes:forward=foo|bar turn:lanes:forward=foo+bar"))
assert lanes.errors.isEmpty()
}
The "lanes:forward=foo|bar" seems like an illegal value. lanes and lanes:forward an lanes:backward should be numeric values. There should be a warning if that is not the case :)
comment:18 by , 11 years ago
To ensure that the test does not throw an exception when the integer conversion fails.
comment:19 by , 11 years ago
Replying to aceman:
So the check could be modified like this:
IF "lanes" exists then:
- IF both "lanes:forward" AND "lanes:backward" exist, check lanes = lanes:forward + lanes:backward
- IF only "lanes:forward" OR only "lanes:backward" exists, check "lanes >= lanes:forward" OR "lanes >= lanes:backward" (the one that exists).
Ignoring the fact that there are lanes dedicated to both driving directions and therefore lanes=lanes:forward+lanes:backward may not be true everywhere isn't really helping. The specification of the lanes key is seriously broken: just have a look at the list of what should be included and what should be excluded when counting - this is far too difficult. lanes=lanes:forward+lanes:backward might be true for your and my region of the world but not for everywhere. Issuing a warning/error would be contra productive for JOSM and OSM, in my opinion.
lanes + lanes:non-motorized = count_of(*:lanes) (for each *) (3rd check proposed by me)
Very bad idea, because there is simply no tag like "lanes:non-motorized". If you want the number of all lanes, you have to count the values of any :lanes-key. It is a well known deficit of the lanes-key that there is no possibility to specify the count of all lanes.
"lanes:non-motorized" was a shorthand for "lanes:foot"+"lanes:bicycle"+"lanes:horse"+etc. But of course if people regularly do not tag those then there is no check possible. I just said above I would expect a "lanes:bicycle" to be there as without it the information of the road is incomplete.
The problem here is that "non-motorized" is an open space. You say it includes foot, bicycle, horse and "etc.". What is this "etc."? This may differ from country to country and maybe will change with time.
follow-up: 21 comment:20 by , 11 years ago
Ah, I always forget there can be lanes in the middle, that are for no direction (blocked) or for both directions. There should be some tag to specify those too (like lanes:both_ways=1) so that the tags add up to "lanes". But if there isn't yet then yes, we can't check anything.
Do we at least agree on "lanes >= lanes:forward" AND "lanes >= lanes:backward" (for whichever key exists) ?
comment:21 by , 11 years ago
Replying to aceman:
Do we at least agree on "lanes >= lanes:forward" AND "lanes >= lanes:backward" (for whichever key exists) ?
One could test for lanes>=lanes:forward+lanes:backward - if lanes:forward/lanes:backward is not set, assume zero. This should work.
follow-up: 24 comment:23 by , 11 years ago
Milestone: | → 14.01 |
---|---|
Resolution: | → fixed |
Status: | reopened → closed |
Thank you!
Enough tests for now, let's not try to define more tests than actually needed at the moment.
comment:24 by , 11 years ago
comment:25 by , 11 years ago
Thanks for listening to us!
The pace of JOSM development is great these days :)
comment:27 by , 11 years ago
comment:28 by , 11 years ago
Year, sure. Before r6647 there was a problem when counting yes||
and similar cases. Now, for the example yes||
, 3 lanes are counted correctly. Higher numbers should work equally well.
Ticket #8520 has been marked as a duplicate of this ticket.