Skip to content

[Translation] Floating number plural broken #30215

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

Closed
kylekatarnls opened this issue Feb 12, 2019 · 18 comments
Closed

[Translation] Floating number plural broken #30215

kylekatarnls opened this issue Feb 12, 2019 · 18 comments

Comments

@kylekatarnls
Copy link
Contributor

Symfony version(s) affected: 4.2.3

Description
Decimal number between 1 and 2 produces incorrect plural in English.

How to reproduce

$t = new \Symfony\Component\Translation\Translator('en');

$t->addLoader('array', new \Symfony\Component\Translation\Loader\ArrayLoader());
$t->addResource('array', [
    'liter' => '%count% liter|%count% liters',
], 'en');

echo $t->trans('liter', ['%count%' => 1.5]);

echo "\n";

$t = new \Symfony\Component\Translation\Translator('fr');

$t->addLoader('array', new \Symfony\Component\Translation\Loader\ArrayLoader());
$t->addResource('array', [
    'liter' => '%count% litre|%count% litres',
], 'fr');

echo $t->trans('liter', ['%count%' => 1.5]);

It outputs:

1.5 liter
1.5 litre

It should output:

1.5 liters
1.5 litre

Possible Solution
int typing here should be removed, it's a float, numbers should not be truncated:

private function getPluralizationRule(int $number, string $locale): int

Related to #16256

@kylekatarnls
Copy link
Contributor Author

Here are some additional sources:
https://english.stackexchange.com/questions/2139/should-we-use-plural-or-singular-for-a-fraction-of-a-mile
https://en.wikipedia.org/wiki/Plural
http://mathforum.org/library/drmath/view/57224.html
http://www.antimoon.com/forum/t1070.htm

All I find tend to define 1.5 as plural in English but Symfony no longer do (it was the cas before the typing was added).

@xabbuh
Copy link
Member

xabbuh commented Feb 13, 2019

Your use case only used to work accidentally based on the fact that we were not able to use scalar type hints on older Symfony versions. The count value always was expected to be an integer. Decimals are currently not supported.

@kylekatarnls
Copy link
Contributor Author

kylekatarnls commented Feb 13, 2019

Before the typing, rules != 1 (English) and <2 (French) were accurate and able to work with floats. Nothing specified decimals were not supported and as far as I know, there were no bugs with floats. So despite it worked accidentally, forcing integer and limit the support to integers was actually a regression.

So unless float causes real problems. It should not be forbidden. If some pluralization rules does not support them, so those specific rules could have int typing, even if it would definitly better to have explicit floor() calls inside rules that need it and float typing instead.

Do you want me to propose a PR?

@xabbuh
Copy link
Member

xabbuh commented Feb 13, 2019

The interface defined that the number had to be an integer:

/**
* Translates the given choice message by choosing a translation according to a number.
*
* @param string $id The message id (may also be an object that can be cast to string)
* @param int $number The number to use to find the index of the message
* @param array $parameters An array of parameters for the message
* @param string|null $domain The domain for the message or null to use the default
* @param string|null $locale The locale or null to use the default
*
* @return string The translated string
*
* @throws InvalidArgumentException If the locale contains invalid characters
*/
public function transChoice($id, $number, array $parameters = [], $domain = null, $locale = null);

Since I have no idea if this works for all supported I would not be in favour of adding this feature.

@kylekatarnls
Copy link
Contributor Author

You can put the int typing inside each plural rule except English which will have float, then change int to float in getPluralizationRule first argument. This way, you fix the English plural with no change in any other language.

Then it can be fixed language per language afterward and it will allow custom plural rule that would now be able to handle floating numbers.

Having correct plural in English seems an important concern for such a great project. Sounds a bit weird to say to symfony users: "Don't use decimal numbers if you wish a correct pluralization".

I have no idea if this works for all supported

Not a reason. This can be tested. I can add tests for 0.5, 1.5 and 2.5 in each language then fix the typing and see what languages changed, so we can check for each language in grammar reference if we have to fix it (as the English plural) or keep the previous rule by forcing int for this particular language.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 24, 2019

Is this a regression or did it always work that way?
Since 4.2, we provide the intl+icu format for messages, which is able to deal with decimals properly.
Could you please give it a try?
See symfony/symfony-docs#10264

@kylekatarnls
Copy link
Contributor Author

@nico-incubiq, it's the result for symfony 4.2, 1.5 liter, it must output 1.5 liters to be linguistically right. As long as you have the int typing, you won't be able to apply both correct French and English rules for decimals (and obviously many others). I will try with the version before the typing was added for comparison if I've some times.

@fabpot
Copy link
Member

fabpot commented Feb 25, 2019

For the record, plural in French starts at 2. So, it's "2 litres" but "1.5 litre". So, nothing is obvious when it comes to languages :)

@kylekatarnls
Copy link
Contributor Author

kylekatarnls commented Feb 25, 2019

Yes @fabpot it's exactly the point. Singular in French is -2 > x < 2, in English, it's x === -1 or x === 1.

Those rules include decimal numbers, so you can't be accurate using int typing.

NB: 1.0 miles is also plural while 1 mile is singular. But I guess this very particular case could be omitted.

@stof
Copy link
Member

stof commented Apr 1, 2019

The question is, does ICU define rules for floating numbers too ? If no, the ICU-based formatting would not help anyway and it is the new recommended default.

@kylekatarnls
Copy link
Contributor Author

kylekatarnls commented Apr 1, 2019

Yes it does (as long as you use groups "few, one, other" of course and not explicit values), this format is used in Angular, and Angular handle properly the plural of 1.5 in French and in English.

1.5 is "one" in French.
1.5 is "other" in English.

As simple.

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@kylekatarnls
Copy link
Contributor Author

No workaround possible, we sill have no way to properly pluralize sentences with floating numbers such as translating "1.5 liters" in English into "1.5 litre" in French.

@carsonbot carsonbot removed the Stalled label Dec 19, 2020
@welcoMattic
Copy link
Member

I just tested this issue, with translations in ICU format, and I do not find the bug.

{{ 'product.carats'|trans({ 'carat': 1.2 }) }}

With this translation:

<unit id="zcajhWf" name="product.carats">
  <segment>
    <source>product.carats</source>
    <target>{carat, plural,
      one   {Weight: # carat}
      other {Weight: # carats}
    }</target>
  </segment>
</unit>

The output is correct:

Weight: 1.2 carats

@kylekatarnls
Copy link
Contributor Author

kylekatarnls commented Jan 12, 2021

The buggy code is still there:

private function getPluralizationRule(int $number, string $locale): int

You need to pass through this method to get the bug and you need 2 languages with different floating pluralization rules.

As long as int $number is in the pluralization rule, 1.2 is rounded down to 1 and so you can't have the proper pluralization for both a language where 1.2 has the same plural as 1 (like French) and an other language where it has the same plural as 2 (like English).

@nicolas-grekas
Copy link
Member

@kylekatarnls would you mind working on a fix please? That's a bugfix for branch 4.4.

@kylekatarnls
Copy link
Contributor Author

Yes, you can assign it to me :)

kylekatarnls added a commit to kylekatarnls/symfony that referenced this issue Jan 18, 2021
kylekatarnls added a commit to kylekatarnls/symfony that referenced this issue Jan 18, 2021
kylekatarnls added a commit to kylekatarnls/symfony that referenced this issue Jan 18, 2021
@kylekatarnls
Copy link
Contributor Author

Fix #39887 submitted.

kylekatarnls added a commit to kylekatarnls/symfony that referenced this issue Jan 18, 2021
kylekatarnls added a commit to kylekatarnls/symfony that referenced this issue Jan 19, 2021
kylekatarnls added a commit to kylekatarnls/symfony that referenced this issue Jan 19, 2021
nicolas-grekas added a commit that referenced this issue Jan 26, 2021
…ekatarnls)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[Translator] fix handling plural for floating numbers

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #30215
| License       | MIT

Commits
-------

533cd7e [Translator] fix handling plural for floating numbers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants