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

Make Decimal comparisons with NaN less arbitrary #46271

Closed
mdickinson opened this issue Jan 31, 2008 · 8 comments
Closed

Make Decimal comparisons with NaN less arbitrary #46271

mdickinson opened this issue Jan 31, 2008 · 8 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@mdickinson
Copy link
Member

BPO 1979
Nosy @rhettinger, @facundobatista, @mdickinson, @tiran
Files
  • richcmp.patch
  • richcmp_signal.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 = 'https://github.com/facundobatista'
    closed_at = <Date 2008-02-07.16:08:14.202>
    created_at = <Date 2008-01-31.04:46:16.127>
    labels = ['type-bug', 'library']
    title = 'Make Decimal comparisons with NaN less arbitrary'
    updated_at = <Date 2008-02-07.16:08:14.200>
    user = 'https://github.com/mdickinson'

    bugs.python.org fields:

    activity = <Date 2008-02-07.16:08:14.200>
    actor = 'mark.dickinson'
    assignee = 'facundobatista'
    closed = True
    closed_date = <Date 2008-02-07.16:08:14.202>
    closer = 'mark.dickinson'
    components = ['Library (Lib)']
    creation = <Date 2008-01-31.04:46:16.127>
    creator = 'mark.dickinson'
    dependencies = []
    files = ['9337', '9340']
    hgrepos = []
    issue_num = 1979
    keywords = ['patch']
    message_count = 8.0
    messages = ['61884', '61885', '61894', '61914', '62110', '62126', '62141', '62155']
    nosy_count = 4.0
    nosy_names = ['rhettinger', 'facundobatista', 'mark.dickinson', 'christian.heimes']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue1979'
    versions = ['Python 3.0']

    @mdickinson
    Copy link
    Member Author

    For Python 3.0 the decimal module got rich comparisons. Those comparisons have somewhat unconventional
    behavior with respect to NaNs, as seen below: <, <= and == comparisons involving NaNs always return False,
    while >, >= and != comparisons always return True.

    The Decimal specification has nothing helpful to say about comparisons involving NaNs. But reading IEEE-
    754r (on which the Decimal specification is closely based), there are two possible options:

    (1) have comparisons involving NaNs (except for !=) raise InvalidOperation in the context, and hence give a
    Python exception (assuming that InvalidOperation isn't trapped.)

    (2) have comparisons involving NaNs always return False (except for !=, which always returns True).

    I think either of these is better than the current behavior. (2) seems like the better option, for a couple
    of reasons: first, it's the way that Python floats currently work, and second, there might be issues with
    list membership testing if equality comparisons involving NaNs raised an exception or returned a NaN.

    Since Mike Cowlishaw is intimately involved with both the Decimal specification and the IEEE-754r process, I
    thought it might be useful to have his opinion on this; his main recommendation was to have the Decimal
    type do the same as the float type.

    The attached patch makes <, <=, >, >= and == comparisons involving NaNs always return False, and !=
    comparisons always return True. It also renames __cmp__ to _cmp and adds a few tests for the new behavior.

    Here's how NaN comparisons currently work:

    Python 3.0a2+ (py3k:60470M, Jan 30 2008, 23:11:40) 
    [GCC 4.0.1 (Apple Inc. build 5465)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> from decimal import *
    >>> n = Decimal("nan")
    >>> i = Decimal("inf")
    >>> n < n
    False
    >>> n > n
    True
    >>> i < n
    False
    >>> i > n
    True

    See also issue bpo-1514428.

    @mdickinson mdickinson added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jan 31, 2008
    @rhettinger
    Copy link
    Contributor

    ISTM, you're now moving far beyond the spec into territory that isn't
    covered by tests or standards. Is this necessary?

    @mdickinson
    Copy link
    Member Author

    Yes, I think it's necessary.

    Speaking of standards, it's the current behavior which isn't backed by any
    standard or rationale other than the historical one involving now-defunct 3-
    way comparisons.

    The proposed behavior is much closer to that specified by C99 (see sections
    7.12.14 and section F.3) and by IEEE 754 and its upcoming revision (see
    section 5.11 of the most recent draft, which is linked to from the wikipedia
    page for IEEE 754r). Even better from a standards-compliance perspective
    would be to have all comparisons except != involving NaNs raise the
    InvalidOperation flag.

    The current behavior also breaks something fundamental: namely, that x < y
    should have the same truth value as y > x. Python even depends on this for
    reversing comparison operators, which explains, but in my opinion doesn't
    excuse, the following oddity:

    >>> 2 < Decimal("NaN")
    True
    >>> Decimal(2) < Decimal("NaN")
    False

    @mdickinson
    Copy link
    Member Author

    Here's a patch (richcmp_signal.patch) that gives an alternative way of doing things: all
    <, <=, > and >= comparisons involving a NaN raise the InvalidOperation flag (using exactly
    the same semantics as those specified for compare_signal). == and != comparisons always
    return False and True, respectively, just as they do now.

    So the post-patch decimal exactly follows IEEE 754(r) on this. I think it makes good sense
    from a user's viewpoint, too: with the default settings, any <, <=, > or >= comparison
    involving a NaN raises a Python exception; this seems like the best outcome for a beginning
    Decimal user, while expert users can choose to use the public compare or compare_signal
    methods instead.

    My worries about list membership testing were nonsensical: there's only one reasonable
    behaviour for == and !=, namely the current behavior, which also matches all the standards.

    On balance, I think I prefer this approach to the idea of returning False on comparisons
    involving NaNs. The only fly in the ointment is that floats and Decimals behave
    differently in this respect.

    @facundobatista
    Copy link
    Member

    I'm +0 regarding this.

    If this will go in, a comment should explicit all this in the code, as
    this behaviour doesn't come from Decimal spec or the PEP.

    Thanks!

    @mdickinson
    Copy link
    Member Author

    Okay: I checked in this change in r60630. The checkin includes comments
    in the code and an extra paragraph describing this behavior in the
    documentation.

    @facundobatista
    Copy link
    Member

    Thanks Mark!

    Shall this issue be closed?

    @mdickinson
    Copy link
    Member Author

    Closing.

    @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
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants