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

gettext: deprecate selecting plural form by fractional numbers #72878

Closed
serhiy-storchaka opened this issue Nov 14, 2016 · 11 comments
Closed

gettext: deprecate selecting plural form by fractional numbers #72878

serhiy-storchaka opened this issue Nov 14, 2016 · 11 comments
Labels
3.7 stdlib type-feature

Comments

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Nov 14, 2016

BPO 28692
Nosy @loewis, @serhiy-storchaka, @timgraham, @zhangyangyu, @JulienPalard
PRs
  • #507
  • Files
  • gettext-non-int-plural-deprecate.patch
  • gettext-non-int-plural-deprecate.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2017-03-24.22:31:54.284>
    created_at = <Date 2016-11-14.18:37:59.518>
    labels = ['3.7', 'type-feature', 'library']
    title = 'gettext: deprecate selecting plural form by fractional numbers'
    updated_at = <Date 2017-03-24.22:31:54.283>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2017-03-24.22:31:54.283>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-03-24.22:31:54.284>
    closer = 'serhiy.storchaka'
    components = ['Library (Lib)']
    creation = <Date 2016-11-14.18:37:59.518>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['45485', '45487']
    hgrepos = []
    issue_num = 28692
    keywords = ['patch']
    message_count = 11.0
    messages = ['280804', '280806', '280810', '280821', '280835', '280839', '280841', '282329', '282331', '282342', '290206']
    nosy_count = 5.0
    nosy_names = ['loewis', 'serhiy.storchaka', 'Tim.Graham', 'xiang.zhang', 'mdk']
    pr_nums = ['507']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue28692'
    versions = ['Python 3.7']

    @serhiy-storchaka
    Copy link
    Member Author

    @serhiy-storchaka serhiy-storchaka commented Nov 14, 2016

    GNU gettext library accepts only integer value for selecting plural form. Python gettext accepts arbitrary numbers. But gettext formulas are not purposed to support non-integer values and can return incorrect result. For example (in Ukrainian):

    "1 площа", but "1.5 площі", not "1.5 площ".
    "2 гектари", but "2.75 гектара", not "2.75 гектарів".
    "5 тонн", but "5.7 тонни", not "5.7 тонн".

    Separate plural form should be used for fractional numbers. Even if fractional part happens to be zero, it is acceptable (e.g. "Time elapsed: 1.000 seconds" in English).

    Proposed patch deprecates fractional numbers for selecting plural form in gettext.

    @serhiy-storchaka serhiy-storchaka added 3.7 stdlib type-feature labels Nov 14, 2016
    @JulienPalard
    Copy link
    Member

    @JulienPalard JulienPalard commented Nov 14, 2016

    Hi, did you forget to attach the patch?

    @serhiy-storchaka
    Copy link
    Member Author

    @serhiy-storchaka serhiy-storchaka commented Nov 14, 2016

    Sorry. Here is a patch.

    @zhangyangyu
    Copy link
    Member

    @zhangyangyu zhangyangyu commented Nov 15, 2016

    Maybe you have forgotten to remove the debug print in the patch?

    @serhiy-storchaka
    Copy link
    Member Author

    @serhiy-storchaka serhiy-storchaka commented Nov 15, 2016

    Yes, of cause. Thank your for noticing this.

    @zhangyangyu
    Copy link
    Member

    @zhangyangyu zhangyangyu commented Nov 15, 2016

    I think the stacklevel should be 3.

    stacklevel = 3:

    ./python -Walways /tmp/a.py
    /tmp/a.py:2: DeprecationWarning: Plural value must be an integer, got float
    c2py('n!=1')(1.1)

    stacklevel = 4:

    ./python -Walways /tmp/a.py
    sys:1: DeprecationWarning: Plural value must be an integer, got float

    @zhangyangyu
    Copy link
    Member

    @zhangyangyu zhangyangyu commented Nov 15, 2016

    Ohh, sorry. It should be 4 and I make a mistake. Sorry for the noise. Patch LGTM.

    @JulienPalard
    Copy link
    Member

    @JulienPalard JulienPalard commented Dec 4, 2016

    But gettext formulas are not purposed to support non-integer values and can return incorrect result

    Looks like the cast to integer is done *before* giving the value to the C gettext expression.

    For example (in Ukrainian)

    As long as gettext expression don't support non-integer values, there exist *no* way to support two languages where plural form differ for non-integer value. Typically if a language A consider 1.5 to be singular and another language B consider 1.5 to be plural, the only place in the code that can make the difference IS in the C plural expression which don't support non-integer values.

    Rouding the value before giving it to ngettext only fixes the issue for the lang of the current developer, as for other languages, translators won't be able to change the rounding properties.

    But should we bet that in most, any, or all languages, 1.5 is considered plural? I think so, according to wikipedia¹: "Plural of nouns typically denote a quantity other than the default quantity represented by a noun, which is generally one."

    So, I think that a better fix than warning for non-integer values may be to round them using math.ceil instead of round. This way we avoid developpers to drop round() everywhere to fix the warning the wrong way, leaving the same bug you're having in Ukrainian.

    ¹: https://en.wikipedia.org/wiki/Plural

    @serhiy-storchaka
    Copy link
    Member Author

    @serhiy-storchaka serhiy-storchaka commented Dec 4, 2016

    round() was used because it is the simplest way to convert fractional numbers to integers that doesn't involve strings-to-integer converting (as in int()). Perhaps math.trunc() looks a little more appropriate. math.ceil() returns a float in 2.7 and is limited by float range. But "1.678 second" don't look more correct than "1.678 seconds". My point is that gettext ability of selecting plural form shouldn't be used for fractional numbers. It seems to me that a fractional number should be formatted with the same form independently from it's value (it can be different from plural forms for integer numbers). Proposed patch adds a deprecating warning for encouraging users to rewrite their code for formatting fractional numbers.

    @JulienPalard
    Copy link
    Member

    @JulienPalard JulienPalard commented Dec 4, 2016

    It seems to me that a fractional number should be formatted with the same form independently from it's value

    Ok, so 1.000 seconds, ok, LGTM.

    Should we mention something explicit about it? Not sure how to write it down, but I still fear that everyone's reaction will be "Oh can't give a float? Let my put a round() in my call" and stay with the same plural bug as before.

    Maybe replace

    Plural value must be an integer

    by

    Should not use ngettext with floating point, consider an invariant form.

    ?

    @serhiy-storchaka
    Copy link
    Member Author

    @serhiy-storchaka serhiy-storchaka commented Mar 24, 2017

    New changeset f659598 by Serhiy Storchaka in branch 'master':
    bpo-28692: Deprecate using non-integer value for selecting a plural form in gettext. (#507)
    f659598

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 stdlib type-feature
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants