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

test_float fails for 'legacy' float repr style #50030

Closed
mdickinson opened this issue Apr 17, 2009 · 8 comments
Closed

test_float fails for 'legacy' float repr style #50030

mdickinson opened this issue Apr 17, 2009 · 8 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@mdickinson
Copy link
Member

BPO 5780
Nosy @mdickinson, @ericvsmith
Files
  • issue5780.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/mdickinson'
    closed_at = <Date 2009-04-17.22:42:45.715>
    created_at = <Date 2009-04-17.10:32:52.831>
    labels = ['interpreter-core', 'type-bug']
    title = "test_float fails for 'legacy' float repr style"
    updated_at = <Date 2009-04-17.22:42:45.713>
    user = 'https://github.com/mdickinson'

    bugs.python.org fields:

    activity = <Date 2009-04-17.22:42:45.713>
    actor = 'mark.dickinson'
    assignee = 'mark.dickinson'
    closed = True
    closed_date = <Date 2009-04-17.22:42:45.715>
    closer = 'mark.dickinson'
    components = ['Interpreter Core']
    creation = <Date 2009-04-17.10:32:52.831>
    creator = 'mark.dickinson'
    dependencies = []
    files = ['13712']
    hgrepos = []
    issue_num = 5780
    keywords = ['patch']
    message_count = 8.0
    messages = ['86066', '86067', '86068', '86070', '86071', '86072', '86074', '86096']
    nosy_count = 2.0
    nosy_names = ['mark.dickinson', 'eric.smith']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue5780'
    versions = ['Python 3.1']

    @mdickinson
    Copy link
    Member Author

    After:

     CC="gcc -DPY_NO_SHORT_FLOAT_REPR" ./configure && make

    test_format_testfile in test_float fails with:

    test.support.TestFailed: Traceback (most recent call last):
      File "Lib/test/test_float.py", line 341, in test_format_testfile
        self.assertEqual(fmt % float(arg), rhs)
    AssertionError: '100000000000.0' != '1e+11'

    The problem is that for str(float), the new formatting code switches to
    exponential notation at 1e11, not 1e12; the legacy code does not.

    A similar issue exists for repr: repr(1e16) should now produce '1e+16'.

    @mdickinson mdickinson self-assigned this Apr 17, 2009
    @mdickinson mdickinson added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Apr 17, 2009
    @mdickinson
    Copy link
    Member Author

    Here's a patch. Eric, do you have time to check this over for general
    sanity?

    @mdickinson
    Copy link
    Member Author

    On second thoughts, I think that repr shouldn't change: 'legacy'
    repr format should mean that it *is* exactly the same as before,
    including the switch at 1e17 instead of 1e16. Otherwise there will be 3
    possibilities for repr output: 'short' repr, 'legacy' repr and pre-3.1
    repr will all be different.

    Here's a new patch, which leaves repr alone and fixes up the Misc/NEWS
    entries accordingly.

    @ericvsmith
    Copy link
    Member

    I agree that there should be 2 behaviors, not 3.

    This patch looks okay to me, and tests pass both with and without
    defining PY_NO_SHORT_FLOAT_REPR.

    One nit: You don't need to use Py_CHARMASK when using the ISDIGIT macro
    locally defined in pystrtod.c. On the other hand, that macro (ISDIGIT)
    should completely go away. I think it's there because this file is
    trying to be locale-unaware, but isdigit from ctype.h is defined as
    being locale-unaware (at least on all platforms I have access to and a
    search on the net; my copy of C89 is at home and I can't triple-check
    just yet).

    @ericvsmith
    Copy link
    Member

    Also, remove_trailing_zeros should go inside the #ifdef so it's not
    included if Gay's code is being used.

    @mdickinson
    Copy link
    Member Author

    I seem to recall that despite the C standards, there are platforms around
    where isdigit and isxdigit are still locale aware. This bit me when I
    implemented float.fromhex: see r65964.

    And my man page for isdigit says:

    COMPATIBILITY
    The 4.4BSD extension of accepting arguments outside of the range of
    the unsigned char type in locales with large character sets
    is considered obsolete and may not be supported in future releases.
    The iswdigit() function should be used instead.

    So I'm tempted to go the other way and say that all uses of isdigit should
    be replaced by ISDIGIT.

    I'll fix the Py_CHARMASK and move remove_trailing_zeros inside the ifdef.

    @ericvsmith
    Copy link
    Member

    I definitely think that if you go with isdigit, you need the macro. I've
    been bitten by this in 3.0.

    My actual suggestion is to go with isdigit and the macro everywhere in
    this file and all files.

    But for just this checkin, ISDIGIT is probably best.

    @mdickinson
    Copy link
    Member Author

    Committed, with the fixups Eric pointed out, in r71692.

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants