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

Inconsistent round behavior between float and int #72123

Closed
JonatanSkogsfors mannequin opened this issue Sep 2, 2016 · 8 comments
Closed

Inconsistent round behavior between float and int #72123

JonatanSkogsfors mannequin opened this issue Sep 2, 2016 · 8 comments
Assignees
Labels
type-bug An unexpected behavior, bug, or error

Comments

@JonatanSkogsfors
Copy link
Mannequin

JonatanSkogsfors mannequin commented Sep 2, 2016

BPO 27936
Nosy @rhettinger, @mdickinson, @jonatanskogsfors
Files
  • fix_round_default_none_with_test.diff
  • 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 = 'https://github.com/rhettinger'
    closed_at = <Date 2016-09-03.08:57:57.222>
    created_at = <Date 2016-09-02.06:52:55.728>
    labels = ['type-bug']
    title = 'Inconsistent round behavior between float and int'
    updated_at = <Date 2016-09-04.18:29:23.444>
    user = 'https://github.com/jonatanskogsfors'

    bugs.python.org fields:

    activity = <Date 2016-09-04.18:29:23.444>
    actor = 'python-dev'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2016-09-03.08:57:57.222>
    closer = 'rhettinger'
    components = []
    creation = <Date 2016-09-02.06:52:55.728>
    creator = 'Jonatan Skogsfors'
    dependencies = []
    files = ['44342']
    hgrepos = []
    issue_num = 27936
    keywords = ['patch']
    message_count = 8.0
    messages = ['274205', '274208', '274212', '274213', '274214', '274265', '274289', '274376']
    nosy_count = 4.0
    nosy_names = ['rhettinger', 'mark.dickinson', 'python-dev', 'Jonatan Skogsfors']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue27936'
    versions = ['Python 3.5', 'Python 3.6']

    @JonatanSkogsfors
    Copy link
    Mannequin Author

    JonatanSkogsfors mannequin commented Sep 2, 2016

    Theo error handling for round is different for float and int.

    Python 3.5.1 (default, Apr 18 2016, 11:46:32) 
    [GCC 4.2.1 Compatible Apple LLVM 7.3.0 (clang-703.0.29)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> round(1.0, None)
    1
    >>> round(1, None)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: 'NoneType' object cannot be interpreted as an integer

    @JonatanSkogsfors JonatanSkogsfors mannequin added the type-bug An unexpected behavior, bug, or error label Sep 2, 2016
    @rhettinger
    Copy link
    Contributor

    The different data types make different choices:

    >>> from decimal import Decimal
    >>> from fractions import Fraction
    >>> (1).__round__(None)
    Traceback (most recent call last):
      File "<pyshell#39>", line 1, in <module>
        (1).__round__(None)
    TypeError: 'NoneType' object cannot be interpreted as an integer
    >>> (1.0).__round__(None)
    1
    >>> Decimal(1).__round__(None)
    Traceback (most recent call last):
      File "<pyshell#41>", line 1, in <module>
        Decimal(1).__round__(None)
    TypeError: optional arg must be an integer
    >>> Fraction(1, 1).__round__(None)
    1
    >>> from _pydecimal import Decimal
    >>> Decimal(1).__round__(None)
    1

    For Fraction and _pydecimal, the behavior comes from using None as a placeholder (which is common and normal in pure python code). For float there is explicit code to test for the None case. For int, the None test was omitted (perhaps a mistake) and the error is raised by PyNumber_Index.

    Looking through tests, only Lib/test/test_float.py tests for None being allowable. Elsewhere, it seems to be an implementation detail.

    The old Python 2 version of the round() function never let None be passed in (because it used PyArg_ParseTupleAndKeywords(args, kwds, "d|i:round"). That logic seems to get lost in the Python 3 version when __round__ was introduced.

    To resolve the differences, I think the round() function should explicitly check for None and replace it with zero before calling the underling __round__ functions where we would allow variable according the needs of the implementation.

    @mdickinson
    Copy link
    Member

    I think the round() function should explicitly check for None and replace it with zero

    That would be a change in behaviour: round(x) is not the same as round(x, 0). For most types, round(x) returns an int, while round(x, 0) returns something of the same type as x. Instead, I think we should add the check for None to ints __round__ implementation.

    >>> round(1.3, 0)
    1.0
    >>> round(1.3)
    1
    >>> round(1.3, None)
    1

    @mdickinson
    Copy link
    Member

    Ah, sorry; now that I've looked at the patch, I see I misunderstood. You're not replacing None with zero; you're replacing it with NULL.

    Patch LGTM.

    @mdickinson
    Copy link
    Member

    The test could be strengthened to add a check that the types of round(x) and round(x, None) match.

    @mdickinson
    Copy link
    Member

    For the record, the float behaviour was changed (for Python 3.5) in http://bugs.python.org/issue19933. There didn't seem to be any particularly strong motivation for that change, but it's done now.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 3, 2016

    New changeset c3c4d8e4ca1a by Raymond Hettinger in branch '3.5':
    bpo-27936: Fix inconsistent round() behavior between float and int
    https://hg.python.org/cpython/rev/c3c4d8e4ca1a

    @rhettinger rhettinger self-assigned this Sep 3, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 4, 2016

    New changeset 7108f2a708c9 by Raymond Hettinger in branch '3.5':
    bpo-27936: Update doc for round() to indicate that None is an allowable argument.
    https://hg.python.org/cpython/rev/7108f2a708c9

    @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
    type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants