Skip to content

Add ceremonial name handling #129

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

Merged
merged 6 commits into from
Jul 20, 2017
Merged

Add ceremonial name handling #129

merged 6 commits into from
Jul 20, 2017

Conversation

bsudekum
Copy link

In cases where we detect a way name is a ceremonial name and not the actual highway name (I-280 vs junipero serra freeway), we should use the ref name.

This is done by inspecting the name and checking and seeing if it includes

["freeway", "expressway", "highway", "fwy", "hwy", "expy"]

/cc @freenerd @willwhite @1ec5

@bsudekum bsudekum requested a review from 1ec5 July 19, 2017 22:00
@bsudekum
Copy link
Author

Tests are failing because there are no translations for the const ceremonialNamesToAvoid.

@@ -41,7 +41,8 @@
"ox": "Keep left",
"xox": "Keep in the middle",
"oxo": "Keep left or right"
}
},
"ceremonialNamesToAvoid": ["freeway", "expressway", "highway", "fwy", "hwy", "expy"]
Copy link
Member

Choose a reason for hiding this comment

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

“Fwy”, “hwy”, and “expy” shouldn’t occur in OpenStreetMap data; any occurrences would require edits to OSM.

index.js Outdated
@@ -139,8 +139,15 @@ module.exports = function(version, _options) {
}
name = name.replace(' (' + step.ref + ')', '');

if (name && ref && name !== ref) {
// In attempt to avoid using the ceremonial name of a way,
Copy link
Member

Choose a reason for hiding this comment

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

I’m not sure where the term “ceremonial name” comes from, but this PR appears to handle two common scenarios:

  • Plain old road names assigned to freeways, like “Northeast Expressway”
  • Memorial highway designations, like “Martin Luther King Junior Memorial Highway” or “CHP Officer David W. Manning Memorial Freeway”

Let’s use the term “highway name” to refer to both senarios.

@@ -41,7 +41,8 @@
"ox": "Keep left",
"xox": "Keep in the middle",
"oxo": "Keep left or right"
}
},
"ceremonialNamesToAvoid": ["freeway", "expressway", "highway", "fwy", "hwy", "expy"]
Copy link
Member

Choose a reason for hiding this comment

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

This is a list of words, not full names.

@@ -41,7 +41,8 @@
"ox": "Keep left",
"xox": "Keep in the middle",
"oxo": "Keep left or right"
}
},
"ceremonialNamesToAvoid": ["freeway", "expressway", "highway", "fwy", "hwy", "expy"]
Copy link
Member

Choose a reason for hiding this comment

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

It’s unlikely that Transifex would be able to handle an array like this well. Even if it does present the array to translators, they’ll have to provide exactly six words to avoid, no more, no less.

We could put this array in the overrides file, but that would shut Transifex entirely out of the process. Or we could join the list together into a single, comma-delimited string, similar to the approach we took in mapbox/mapbox-navigation-ios#254.

index.js Outdated
// In attempt to avoid using the ceremonial name of a way,
// check and see if the name includes strings like `highway` or `expressway`.
// If this is true and there is a ref, use the ref.
var nameIsCeremonialName = instructions[language][version].constants.ceremonialNamesToAvoid.some((stringToAvoid) => name.includes(stringToAvoid));
Copy link
Member

Choose a reason for hiding this comment

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

It would be far better to look at the previous step’s classes for motorway than to rely on a word appearing in the road name. For one thing, “highway” is often used for surface streets, and even sometimes “expressway”. Moreover, in some countries, there simply aren’t good stop words that apply to only freeways.

index.js Outdated
// the ref should be used instead of the name.
var wayIsOnHighway = false;
if (options && options.classes) {
wayIsOnHighway = options.classes.some((className) => ['toll', 'highway'].indexOf(className) > -1);
Copy link
Member

Choose a reason for hiding this comment

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

A toll road isn’t necessarily a nameless road. In fact, I’d argue that toll roads in the U.S. are more identifiable by their names than by their route numbers, even though OpenStreetMap tends to put acronyms in ref for map display purposes. Consider:

For the most part, these toll roads are already tagged as motorways. However, Chickasaw Turnpike is tagged as a trunk; it would be better to call it “Chickasaw Turnpike” than “OK 7 Spur”.

More broadly, we shouldn’t give so much prominence to purely alphabetic refs, which may be mere abbreviations of the full name. For example, all the New York Parkways have acronyms in ref: it should be “Saw Mill River Parkway”, not “SMP”.

Given these examples, I think we should use the following logic, at least until we’re able to unambiguously identify routes based on route relations:

  1. If the classes contain motorway (highway?) and the ref contains a digit, say just the ref.
  2. If the classes contain motorway and the ref contains no digit, say only the name.
  3. If the classes don’t contain motorway, say the name followed by the ref.

If you’re concerned that we’d be tossing out too many refs by requiring a digit, we could instead check whether the ref is a single, all-caps word – so GSP, not Orange.

I think these heuristics would work well for North America. Perhaps @freenerd could comment on whether they make any sense in Europe.

@willwhite
Copy link
Contributor

I thought the plan was to use the new classes from OSRM to determine whether we should use name or ref. Did that turn out to be impossible?

@bsudekum
Copy link
Author

Noting, updated this to use the new classes key in the directions api.

wayName = name + ' (' + ref + ')';
} else if (name && ref && wayMotorway && (/\d/).test(ref)) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: drop the extra parentheses around the regular expression.

Copy link
Author

Choose a reason for hiding this comment

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

The linter for this project actually requires them 😅

@bsudekum bsudekum merged commit c3fae5f into master Jul 20, 2017
@bsudekum bsudekum deleted the ceremonial branch July 20, 2017 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants