Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds barrier=gate regression tests, see #581 #3445

Merged
merged 1 commit into from
Dec 15, 2016
Merged

Conversation

daniel-j-h
Copy link
Member

See #581 and the recent discussion in #3435.

These tests all pass even though barrier=* implies access=no per Wiki.

But we still route e.g. over barrier=gate without any further access tags.

Scenario: Car - barrier=gate should be routed over unless explicitely forbidden
Then routability should be
| node/barrier | access | bothw |
| gate | | x |
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the questionable default.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So shouldn't it be the other way around? If you want to add the test, it should be as a todo and have the correct default value. Otherwise it might imply to someone that we want it to be this way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want it to be this way. The test is just being explicit about this behavior.

Strictly speaking we're going against the Wiki's recommendation to not route over gates without any access tags. The reason I'm adding this (passing) test is to make a point for #3435.

Where data in the wild trumps the Wiki's recommendation by a huge amount of data points we can no longer call it "mis-tagging". Instead we should support what's there.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 misunderstood the questionable default part here. Thought you where referring to what you added as strange.

@emiltin
Copy link
Contributor

emiltin commented Dec 13, 2016

well. i've seen barrier=gate more than once in places where access is indeed possible, like https://www.openstreetmap.org/edit?node=1227080468#map=19/55.69242/12.55155

@TheMarex
Copy link
Member

This needs a rebase, current conflicts in master.

@daniel-j-h daniel-j-h merged commit 94854b5 into master Dec 15, 2016
@MoKob MoKob deleted the gate-routability branch December 15, 2016 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants