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

Fix guidance sliproads scenario 3 #4493

Merged
merged 4 commits into from
Oct 9, 2017
Merged

Conversation

oxidase
Copy link
Contributor

@oxidase oxidase commented Sep 11, 2017

Issue

PR fixes scenario 3 of #4348
screenshot from 2017-09-11 20-00-06

by adding an additional check for an intermediate node

Also PR changes allowed angle from 115 to 140 degrees for non-link sliproad check that updates scenario 2 of #4348

Tasklist

  • add regression / cucumber cases (see docs/testing.md)
  • review
  • adjust for comments

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

Copy link
Member

@daniel-j-h daniel-j-h left a comment

Choose a reason for hiding this comment

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

Can you make maps for before / after, e.g. for Berlin and SF. I would love to understand what will change here with this addition. I think you can use the geojson logger we have for this use-case.

Also do you think we should always only skip a single intersection? Or should we skip intersections until a distance threshold is reached?

@daniel-j-h
Copy link
Member

Travis fails: dk:rural seems not to be in taginfo.json.

I don't know why we didn't see this before.

@oxidase oxidase force-pushed the guidance/sliproads-scenario3 branch 2 times, most recently from 923c43a to ad9ef92 Compare September 12, 2017 19:17
@oxidase
Copy link
Contributor Author

oxidase commented Sep 12, 2017

@daniel-j-h i have created a map where

As i see blue nodes is really big change, so i think to remove

if (deviation_from_straight > 180. - minimal_crossroad_angle_of_intersection &&
condition but it will require adding _link in OSM sliproad ways to keep some "green" sliproad nodes.

@oxidase oxidase force-pushed the guidance/sliproads-scenario3 branch from ad9ef92 to 1ab127b Compare September 13, 2017 12:33
@daniel-j-h daniel-j-h requested a review from MoKob September 27, 2017 22:26
@oxidase oxidase force-pushed the guidance/sliproads-scenario3 branch from 1ab127b to 8f83192 Compare October 2, 2017 16:54
@oxidase oxidase added this to the 5.13.0 milestone Oct 2, 2017
@oxidase oxidase force-pushed the guidance/sliproads-scenario3 branch from 8f83192 to d6974cb Compare October 5, 2017 12:01
Copy link
Member

@daniel-j-h daniel-j-h left a comment

Choose a reason for hiding this comment

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

Just sampled some locations affected by this change and it looks good from what I've seen so far. 👍

@TheMarex TheMarex merged commit 837dba2 into master Oct 9, 2017
@TheMarex TheMarex deleted the guidance/sliproads-scenario3 branch October 9, 2017 14:35
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.

3 participants