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

Improving Swedish translation (Exit numbers, overrides, modifiers and roundabouts and questions) #136

Closed
davols opened this issue Aug 21, 2017 · 21 comments

Comments

@davols
Copy link
Contributor

davols commented Aug 21, 2017

I'm currently working in transifex. And having some issues and questions regarding the best way to handle the translations.

Overrides


`delete content.v5.turn['sharp right'];` 

~~~It seems better to use overrides to fix this (to remove `sharp` and `slight`, same for `continue` as well. Tried to have an override similar to the example above but running the `UPDATE=1 npm test` still produce turn sharp left fixtures.~~~


~~~Would I be using overrides correctly with the example above? And would it in that example just fall back to `v5.turn.right` when `sharp.right` comes up?~~~ Fixed in transifex by just translating `slight left` et al to `left` (note the uturn issue for turns)

Note for some docs: Explain the overrides a bit, preferably with examples (saw German has a several overrides)

### Exit numbering
~~~Exit numbering for highways, ignores the exit number and only uses "towards ..." Transifex is saying every line is translated. This seems to be missing? (to translate those strings)~~~ Fixed in transifex


### "extra modifiers"
~~~Related to overrides, there are several situations where one wants to remove the sharp/slight. But there are also examples where another preposition should be added between `sharp` and `left`. Never used transifex before and not sure if this is done easily.~~~ Removed the adjectives
~~~Like `turn 1{modifier:1} to 1{modifier:2}` (where `modifier1` would be sharp or left and `modifier2` right/left.~~~

Is this possible? Otherwise the "extra" modifiers (sharp/slight) should probably be removed. 

### Roundabout & Rotary
~~~Have some thoughts about the translation and also shortening it.~~~

~~~For instance the text "Enter the roundabout and take the 2nd exit towards" could be shorten. One way to translate and shorten it is "In the roundabout take the 2nd exit towards"~~~

~~~Similar for rotary which right now is "Drive into the roundabout" (in Swedish being quite literal) which could be changed to "In the roundabout"~~~ Shortened and improved in transifex

~~~Not sure about the thoughts about things like that and also where they should be changed (transifex vs override) - They would make more sense but I'm also afraid they will be incorrect for some small use cases.~~~ For Swedish they are in transifex now (for sharp and slight). 
@1ec5
Copy link
Member

1ec5 commented Aug 23, 2017

@davols, I just noticed this ticket – do you think I should include Swedish in #137 or wait until some of these issues are sorted out?

@1ec5
Copy link
Member

1ec5 commented Aug 23, 2017

Tried to have an override similar to the example above but running the UPDATE=1 npm test still produce turn sharp left fixtures.
Maybe it always does but not in production/usage of the language file?.

Did you execute npm run transifex after adding the override file? I think this line in the transifex script applies the override file.

Exit numbering for highways, ignores the exit number and only uses "towards ..." Transifex is saying every line is translated. This seems to be missing? (to translate those strings)

There should be separate strings for cases with and without exit numbers. In #137, the off_ramp/slight_right_exit_destination test fixture, for example, reads, “Ta 4A avfarten till höger mot Destination 1” – is that correct?

Related to overrides, there are several situations where one wants to remove the sharp/slight. But there are also examples where another preposition should be added between sharp and left.

Unfortunately, I don’t think it’s possible with the JSON format that this project imports into Transifex. #101 tracks extending the format to allow for token variants, although I wondered in #19 whether we should replace JSON with a more robust format.

/cc @frederoni

@davols
Copy link
Contributor Author

davols commented Aug 23, 2017

@1ec5 Swedish in #137 would that include changing (or adding them for my view, since I only have access to Swedish) in transifex? I think it can be added to that PR (or I could make a PR to the project for updated Swedish)

Did you execute npm run transifex after adding the override file? I think this line in the transifex script applies the override file.

Should say I did not, most of the runs were without npm run transifex (thought it was updated when running the update script)

There should be separate strings for cases with and without exit numbers. In #137, the off_ramp/slight_right_exit_destination test fixture, for example, reads, “Ta 4A avfarten till höger mot Destination 1” – is that correct?

Looks close enough, the positioning of the exit number and the rest of the text (for roundabouts etc) is incorrect right now but changed in my/our transifex setup at the moment (As in, 4A should be after 'avfarten' and the word avfarten should be changed as well). Is that string a new one in transifex (related to #137?) that will be added or is it based on the roundabout/rotary exit numbering and texts?

Unfortunately, I don’t think it’s possible with the JSON format that this project imports into Transifex. #101 tracks extending the format to allow for token variants, although I wondered in #19 whether we should replace JSON with a more robust format.

If overrides, running npm run transifex before updating the strings works as I think it does, removing those attributes (sharp/slight) I don't think this would be an issue anymore. Adding a preposition to instructions (mostly continue and turns) between the direction and attribute is a fix - but the better fix in a language sense would be to remove the attributes.

Overall I think it can be added to #137, I'll talk with some people tomorrow regarding the translations and test it a bit. But an updated sv.json isn't too far away (days)

The shortening roundabouts could either be done in transifex and we (not osrm) could test it, short it straight away or leave it for now.

Edit:
Took another look in transifex. For example
Take exit {exit} on the right (key: v5.off ramp.right.exit) could differ depending on if the exit is a numbering (like 1st, 2nd etc) or the name/numbering (4A, 113) in Swedish. Glossary confuses me a bit right now, would that String w/ key always be an actual number (4a, 113 etc) and not a counting number (First, 2nd etc)

@1ec5
Copy link
Member

1ec5 commented Aug 23, 2017

#137 is pulling in the latest translations from Transifex, so any changes you make there will be reflected in the PR before it gets merged. Other than potentially adding an override file, any changes you make to the localization would have to take place in Transifex; otherwise they’d get overridden sometime in the future.

Took another look in transifex. For example
Take exit {exit} on the right (key: v5.off ramp.right.exit) could differ depending on if the exit is a numbering (like 1st, 2nd etc) or the name/numbering (4A, 113) in Swedish. Glossary confuses me a bit right now, would that String w/ key always be an actual number (4a, 113 etc) and not a counting number (First, 2nd etc)

The {exit} token, which appears in off ramp instructions, is always a signposted exit number, such as 4A or 113. The {exit_number} token, which appears in rotary and roundabout instructions, is always an ordinal number like 1st or 2nd. I realize this is confusing and poorly documented; the token names are based on the fields of the RouteStep and StepManeuver objects in OSRM responses.

@davols
Copy link
Contributor Author

davols commented Aug 24, 2017

#137 is pulling in the latest translations from Transifex, so any changes you make there will be reflected in the PR before it gets merged. Other than potentially adding an override file, any changes you make to the localization would have to take place in Transifex; otherwise they’d get overridden sometime in the future.

Sounds like we should use overrides. Perhaps it's easier to create a separate PR.

extra modifiers/remove slight and sharp

I've added several overrides (sv.json):

    delete content.v5.turn['slight left'];
    delete content.v5.turn['slight right'];
    delete content.v5.turn['sharp left'];
    delete content.v5.turn['sharp right'];

Ran npm run transifex and then UPDATE=1 npm test and the fixtures still included the incorrect instructions. Example

  "step": {
    "maneuver": {
      "type": "turn",
      "modifier": "sharp left"
    },
    "name": ""
  },

We want this to just fall back to turn left, because the actual string created by the modifier is incorrect (and needs a preposition between sharp and left) so it's easier to just remove it and use turn left
Am I missing something or is overrides not used in the fixtures but in actual instructions in the project? If fixtures ignores it and the overrides will work I would gladly add the overrides in a PR with updated language file.

Another way would be to translate the string sharp left etc to just left. Not sure about the best way to change these (overrides vs translate that string).

Use exit names for turns too?

I've fixed the off ramp with exit number in Transifex right now. And at the same time I found another interesting question/issue.

"maneuver": {
      "type": "turn",
      "modifier": "left"
    },
    "name": "Way Name",
    "exits": "4A;4B"
  },

This creates a string only based on name and it doesn't seem to exist one that uses the exits. Would it not be better if exit exists to use that also for turns? I understand to use name if it exists, but if exits also exists I think that creates a better instruction for the user.

@davols
Copy link
Contributor Author

davols commented Aug 24, 2017

Found some extra issues in regards to modifiers.

When it comes to type turn the modifier uturn is also incorrect (basically says Turn U-turn instead of make a u-turn).

Is it possible to remove modifiers sharp right to slight left for type turn and then translate the default v5.turn.default.default to "Make a" such as v5.turn.default.default is only used for uturn?

So that all turn (except uturn) uses either turn left and turn right? (Make a slight left should just become turn left)
Understand how to translate that string in transifex, but not how to override/force all turn with sharp and slight modifiers to just drop the slight/sharp bit and keep the direction part of the modifier.

Or possibly "extract" and make a special translation for uturn turns (as they exist for pretty much all other types.

Apart from the issue with turn and uturn (I guess exit names for turns too should be another issue) transifex has a better updated Swedish translation now. Still wondering about overrides as mentioned but it could be added into #137 cc @1ec5

Not making (example, since it's 4 possibilities)turn sharp right forced to use/become v5.turn.right OR adding another key/string for turn.uturn makes it impossible to have a great translation for turns.

@frederoni
Copy link
Contributor

I agree that it sounds a bit ungrammatical, and it's rare to use slight and sharp in conjunction with turns in Swedish, but it's not always left out:

screenshot 2017-08-24 11 46 12

Would it make sense just to omit the adjective from the translation and skip the overrides? It's roughly 20 strings that would look the same but we keep the options open.

@davols
Copy link
Contributor Author

davols commented Aug 24, 2017

Would it make sense just to omit the adjective from the translation and skip the overrides? It's roughly 20 strings that would look the same but we keep the options open.

If I'm understanding you right it has been one of the thoughts we've had (basically translate the slight left to just left) for those strings. I wasn't sure if any of those solutions (overrides vs removing the adjective in the translation) would be frowned upon and I assumed (yup, assumed) that it would be seen as cleaner doing it in overrides. Removing the adjectives in the current transifex translation would make it really good, but not perfect (see turn uturn - sväng u-sväng for turn.uturn)

Also removed the preposition (åt) for turns in Transifex. Which, if the adjectives are to be used has to happen as well.

@frederoni
Copy link
Contributor

frederoni commented Aug 24, 2017

Ah, I think I found the issue.

EN "turn": { "default": { "default": "Make a {modifier}",
SV "turn": { "default": { "default": "Sväng {modifier}",

turn.default.default in Swedish should be "Gör en".

@davols would you mind creating a PR so we can continue working on the translations line by line there?

@davols
Copy link
Contributor Author

davols commented Aug 24, 2017 via email

@1ec5
Copy link
Member

1ec5 commented Aug 24, 2017

Thanks @davols! In the meantime, it sounds like it’s safe to land #137 with the latest translation changes from Transifex while we await other improvements in a separate PR.

@davols davols mentioned this issue Aug 25, 2017
@davols
Copy link
Contributor Author

davols commented Aug 25, 2017

@1ec5 I created another PR for Swedish from Transifex.

Unless every turn.modifier (with adjectives) (except for uturn) can be linked with turn.left or turn.right then it's impossible to have a correct version for all cases. But having an incorrect turn.uturn is way better than to have 5 clashing turns.

@frederoni Had another talk with a person who knows more Swedish than me said it's great to just remove the adjectives. So in #138 they are removed in the translation of slight left etc (and overrides are not used)

In regards to my comment about using exits for turns too. I guess/wonder it should be another issue. Similar to #128

@davols
Copy link
Contributor Author

davols commented Aug 31, 2017

@1ec5 I'm not sure what you (the maintainers) would prefer. Edited comments or new comments.

I'm quite curious about the changelog.

0.5.0 2017-07-07

Support exit property on off ramp type
Curently supported languages: english, vietnamese, german

Are these fixed now with the updated Swedish (for Swedish) or is there anything I can help out with?

Another open question (for several different maneuver but I'll use turn as the example) the exit is ignored, by looking at the test fixtures, (at least when there is a way_name. Is this a decision? Not sure if these exist in real life (where turn would have exit info. I really don't want to spam this issue, so are there any other platforms for these types of questions?
(I guess the ongoing discussion regarding definitive form and more specific turn strings is also something that might be more suitable on another forum)

@1ec5
Copy link
Member

1ec5 commented Aug 31, 2017

Are these fixed now with the updated Swedish (for Swedish) or is there anything I can help out with?

Those should be fixed now. v0.5.0 is an older version that supported exit instructions only for those three initial languages, since those were the only languages that had been updated at the time. As of v0.5.3, Swedish exit instructions should be fine (“Improves the wording of various instructions in … Swedish”).

Another open question (for several different maneuver but I'll use turn as the example) the exit is ignored, by looking at the test fixtures, (at least when there is a way_name. Is this a decision? Not sure if these exist in real life (where turn would have exit info.

I can’t find any examples off the top of my head, but I may’ve seen examples occasionally while traveling in the U.S. Freeway entrances with destinations are common here, but OSRM Text Instructions should already handle them correctly. I can think of two problems with handling other kinds of maneuvers:

  • OSRM would need to emit an exit number for the turn, but if I understand correctly, the routing graph preserves only edges, not vertices. So the exit number would need to be the same whether you’re going northbound and turning right or going southbound and turning left.
  • In OpenStreetMap, destination can occur on any street, but we normally only want to incorporate it into the instruction when there’s entrance/exit-like signage. In the U.S., it isn’t uncommon for destination to be placed on the main motorway to indicate where you’d end up if you don’t take any exits, but we may not want to incorporate that destination in any instruction.

If you can find a good example of a standard turn that has an exit number and freeway-style guide signage, please open a new issue about it, since that would apply to all languages, not just Swedish. Then we can discuss possible ways to resolve the issues above. Please link to a relevant OpenStreetMap feature or a photo if you can. Thanks!

@1ec5
Copy link
Member

1ec5 commented Sep 1, 2017

I found some examples of turns that should have exit numbers. Let’s track that in Project-OSRM/osrm-backend#4467.

@davols
Copy link
Contributor Author

davols commented Sep 13, 2017

@1ec5 I revisited transifex today and saw a bunch of new untranslated strings. Some I want to say were translated before (no history) but also some new ones, for example with ref (v5.phrase.name and ref)

Is there a nice way to keep updated when things are changed or added to transifex so we can work with updating new ones?

@1ec5
Copy link
Member

1ec5 commented Sep 13, 2017

Yes, there’s a tiny Watch button in the top-right corner of the dashboard. You’ll get an e-mail every time en.json changes in this repository.

watch

@davols
Copy link
Contributor Author

davols commented Sep 13, 2017

The new untranslated strings and updates, would you want a new PR for new Swedish translated strings or is it sufficient with a ping in this issue?

@1ec5
Copy link
Member

1ec5 commented Sep 13, 2017

By sheer coincidence, #154 is going to pull in the Swedish changes. In general, a PR is most convenient for the project maintainers, but a ping is still helpful.

@1ec5
Copy link
Member

1ec5 commented Sep 28, 2017

The changes in #154 eventually made their way into iOS navigation SDK v0.8.0 last week, and we’re working on getting them deployed as part of the Directions API for the other platforms. With respect to OSRM Text Instructions proper, are there any other changes we need to take care of for Swedish, aside from routine translation updates? If not, we can close this issue and track ongoing translation updates in PRs.

@davols
Copy link
Contributor Author

davols commented Sep 29, 2017

With respect to OSRM Text Instructions proper, are there any other changes we need to take care of for Swedish, aside from routine translation updates? If not, we can close this issue and track ongoing translation updates in PRs.

Not by the looks of it no. Closing it now since all the issues has been addressed and fixed.

@davols davols closed this as completed Sep 29, 2017
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

No branches or pull requests

5 participants
@frederoni @1ec5 @davols @hdaymon and others