Grammar support for Russian way names #102

yuryleb · 2017-04-06T16:09:10Z

New feature for OSRM Text Instructions - grammatical cases support for way names. This is quite important for many languages (Russian, Ukrainian, Polish, Bulgarian, Finnish, Estonian and many others) - otherwise translated strings with inserted way names (in nominative case as in OSM) look grammatically incorrect in most cases.

This feature extends {way_name} and {rotary_name} in instructions with optional grammar case label and so requires update of other applications used OSRM Text Instructions data (Ruby #26 and Swift #53).

Unfortunately this proposal is incompatible with Transifex (see #101) and so can't be easily merged. This PR is supposed mostly for proposed changes review in details and to illustrate this problem for public.

freenerd · 2017-04-10T09:50:20Z

Thanks @yuryleb. As mentioned before, we need to do some more thinking here.

Blockers seem to be:

Re-think localization file format Make instructions.json format localizable #19
Fix workflow with transifex to allow for different tokens

yuryleb · 2017-09-27T08:42:42Z

@freenerd, @1ec5, @oxidase, finally I solved to add grammatical case labels in new languages\overrides\ru.js script to post-process Transifex localization data - translated strings parsing seems quite simple and could be easily patched for new context phrases or even re-used for other languages if it will be necessary.

I don't think Transifex or any other localization service will allow arbitrary keywords expansion. Actually languages\overrides post-processing used by OSRMTI for final customization looks the better idea.

So let's meet the first route instructions with proper grammatical cases at least in Russian 🎉

BTW other OSRMTI-related projects (osrm-text-instructions.swift, osrm-text-instructions.java, etc?) should also be updated to expect {way_name:*} and {rotary_name:*} variables at least or completely support grammatical cases (with way described in Grammar.md) at last.

1ec5

Thanks, I like the general approach. It looks like there's a to-do or two in here, so I'll hold off on a final review until you're ready.

1ec5 · 2017-09-29T18:11:27Z

Grammar.md

+
+Many languages - all Slavic (Russian, Ukrainian, Polish, Bulgarian, etc), Finnic (Finnish, Estonian) and others - have [grammatical case feature](https://en.wikipedia.org/wiki/Grammatical_case) that could be supported in OSRM Text Instructions too.
+Originally street names are being inserted into instructions as they're in OSM map - in [nominative case](https://en.wikipedia.org/wiki/Nominative_case).
+To be grammatically correct, street names should be changed according to target language rules and instruction context before insertion.


Are the various cases always regular and easy to determine from the nominative case? I noticed a few features have been tagged name:dative in OpenStreetMap – would it be desirable or necessary for OSRM to pass such tags along to OSRM Text Instructions?

IMHO it's not deal of OSM - it's impossible to store all case variants in all languages for each name. Perhaps CLDR will be the better place (if will 😉 )

1ec5 · 2017-09-29T18:13:33Z

Grammar.md

+
+- Russian regular expressions are based on [Garmin Russian TTS voices update](https://github.com/yuryleb/garmin-russian-tts-voices) project; see [file with regular expressions to apply to source text before pronouncing by TTS](https://github.com/yuryleb/garmin-russian-tts-voices/blob/master/src/Pycckuu__Milena%202.10/RULESET.TXT).
+- There is another grammar-supporting module - [jquery.i18n](https://github.com/wikimedia/jquery.i18n) - but unfortunately it has very poor implementation in part of grammatical case applying and is supposed to work with single words only.
+- Actually it would be great to get street names also in target language not from default OSM `name` only - there are several multi-lingual countries supporting several `name:<lang>` names for streets. But this the subject to address to [OSRM engine](https://github.com/Project-OSRM/osrm-backend) first.


This is a good idea – please file an issue in osrm-backend.

1ec5 · 2017-09-29T18:14:59Z

Grammar.md

+```
+- All such JSON files should be registered in common [languages.js](languages.js)
+- Instruction text formatter ([index.js](index.js) in this module) should:
+  - check `{way_name}` and `{rotary_name}` variables for optional grammar case after colon: `{way_name:accusative}`


I like this syntax – it leaves open the possibility of additional transformations like capitalization, which could replace the "capitalizeFirstLetter" meta property currently set on some localizations.

1ec5 · 2017-09-29T18:16:58Z

Grammar.md

+
+### Notes
+
+- Russian regular expressions are based on [Garmin Russian TTS voices update](https://github.com/yuryleb/garmin-russian-tts-voices) project; see [file with regular expressions to apply to source text before pronouncing by TTS](https://github.com/yuryleb/garmin-russian-tts-voices/blob/master/src/Pycckuu__Milena%202.10/RULESET.TXT).


Thank you for dedicating this rule set to the public domain!

1ec5 · 2017-09-29T18:29:01Z

index.js

+                var rules = grammars[language][version][grammar];
+                if (rules) {
+                    // Pass original name to rules' regular expressions enclosed with spaces for simplier parsing
+                    var n = ' ' + name + ' ';


It's currently possible for clients to manipulate a step's name before compilation. For example, the Mapbox Directions API currently inserts SSML markup around numbers within the name so that speech synthesizers pronounce them more casually. I think that could potentially interfere with this feature. That would be a good argument for implementing #52, so that clients can reliably manipulate the name after grammaticalization.

No problem, if clients will not damage {way_name:*} vars - for example, "Restore names highlighting" PR adds HTML tags around these vars and all works as on snapshot above.
But if Mapbox Directions inserts SSML tags right instead {way_name}, this looks wrong - for example, Nuance uses \tn=address\ tags in its Vocalizer voices to specify start of address text specially to pronounce numbers and known street abbreviations as address but doesn't change original address/name text.

But actually it's no problem, even if Mapbox will catch and replace {way_name} and skip {way_name:accusative} etc - this allows further "grammarization" to work 😉 SSML tags are actually important only for English addresses, Russian & many others have no special requirements for them.

I’m referring to a JavaScript port of mapbox/mapbox-navigation-ios#552 for the Directions API’s (not yet documented) voice_instructions parameter. Essentially we’re working around a problem where Amazon Polly assumes an address is abbreviated, whereas OSM never abbreviates road names. The workaround is to wrap only numbers in <say-as interpret-as="address"> tags, since we still want numbers to be optimized for speech even if the rest of the words can’t be treated as part of an address. For the JavaScript port, we’re changing the value of way_name itself, so that by the time compile() runs, way_name contains SSML code. 😬

I’ll be the first to admit it’s a hack, but I wanted to point this out because clients may be inclined to do similar things to the value of way_name unless we advise against doing so in this project’s documentation. If this PR forces us to implement #52, that’d be a wonderful outcome in my opinion. 😉

/cc @allierowan

1ec5 · 2017-10-03T23:02:01Z

index.js

+                    // Pass original name to rules' regular expressions enclosed with spaces for simplier parsing
+                    var n = ' ' + name + ' ';
+                    rules.forEach(function(rule) {
+                        var re = new RegExp(rule[0]);


Should these rules be case insensitive?

Actually no (and no for g option for all replaces not only for the first match), at least for Russian OSM street names this is unnecessary - there are many working street names validators those keep them compliant to Russian street names agreement. And also all current grammar regular expressions take into account potentially capital first word letter.

But for general case it looks useful to add some meta block into grammar JSON to specify required regular expression options explicitly.

1ec5 · 2017-10-03T23:03:02Z

test/grammar_test.js

+        ['Третье Транспортное кольцо', 'genitive', 'Третьего Транспортного кольца'],
+        ['Третье Транспортное кольцо', 'prepositional', 'Третьем Транспортном кольце']
+    ]
+    // TODO add your language grammar tests to call grammarize() and check result


This is what the code below does, right?

This is the place to add other language's test strings to check other language's grammar when it will be.

1ec5 · 2017-10-04T18:27:07Z

Thanks @yuryleb! I’m going to merge this PR now, but feel free to iterate on the feature in separate PRs if you have any additional changes to make.

bsudekum requested review from 1ec5 and freenerd April 6, 2017 16:10

yuryleb force-pushed the grammar branch from 4039d6f to f4f7a30 Compare June 19, 2017 20:40

hdaymon added translations and removed translations labels Sep 7, 2017

yuryleb force-pushed the grammar branch from f4f7a30 to 164fae6 Compare September 8, 2017 07:07

yuryleb force-pushed the grammar branch 3 times, most recently from 28e2300 to 3eb40ee Compare September 16, 2017 19:09

yuryleb force-pushed the grammar branch from 3eb40ee to a381b3e Compare September 25, 2017 19:58

Add grammar support for Russian way names

2991bc5

yuryleb force-pushed the grammar branch from a381b3e to 2991bc5 Compare September 27, 2017 08:12

yuryleb changed the title ~~[WIP] Grammar support for way names (for Russian instructions as example)~~ Grammar support for Russian way names Sep 27, 2017

1ec5 reviewed Sep 29, 2017

View reviewed changes

yuryleb mentioned this pull request Sep 30, 2017

Return way names in all available languages Project-OSRM/osrm-backend#4561

Open

This was referenced Oct 2, 2017

Callback for token replacement values in compile #52

Closed

expose getWayName from compile for other use cases #167

Merged

1ec5 approved these changes Oct 3, 2017

View reviewed changes

Extend grammar JSON with meta.regExpFlags option

044836a

1ec5 merged commit 3e701e9 into Project-OSRM:master Oct 4, 2017

yuryleb deleted the grammar branch October 4, 2017 20:20

yuryleb mentioned this pull request Oct 4, 2017

Can't extend {way_name} with an option on Transifex #101

Closed

This was referenced Oct 5, 2017

Correct german counting of multiple waypoints #112

Open

Grammar Project-OSRM/osrm-text-instructions.swift#48

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Grammar support for Russian way names #102

Grammar support for Russian way names #102

yuryleb commented Apr 6, 2017

freenerd commented Apr 10, 2017

yuryleb commented Sep 27, 2017

1ec5 left a comment

1ec5 Sep 29, 2017

yuryleb Sep 30, 2017 •

edited

Loading

1ec5 Sep 29, 2017

1ec5 Sep 29, 2017

1ec5 Sep 29, 2017

1ec5 Sep 29, 2017

yuryleb Sep 30, 2017

yuryleb Sep 30, 2017 •

edited

Loading

1ec5 Oct 2, 2017 •

edited

Loading

1ec5 Oct 3, 2017

yuryleb Oct 4, 2017

1ec5 Oct 3, 2017

yuryleb Oct 4, 2017

1ec5 commented Oct 4, 2017


		### Notes

		- Russian regular expressions are based on [Garmin Russian TTS voices update](https://github.com/yuryleb/garmin-russian-tts-voices) project; see [file with regular expressions to apply to source text before pronouncing by TTS](https://github.com/yuryleb/garmin-russian-tts-voices/blob/master/src/Pycckuu__Milena%202.10/RULESET.TXT).

Grammar support for Russian way names #102

Grammar support for Russian way names #102

Conversation

yuryleb commented Apr 6, 2017

freenerd commented Apr 10, 2017

yuryleb commented Sep 27, 2017

1ec5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuryleb Sep 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuryleb Sep 30, 2017 • edited Loading

Choose a reason for hiding this comment

1ec5 Oct 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 commented Oct 4, 2017

yuryleb Sep 30, 2017 •

edited

Loading

yuryleb Sep 30, 2017 •

edited

Loading

1ec5 Oct 2, 2017 •

edited

Loading