Skip to content

Broken tests with "legacy" sys.float_repr_style #128051

Closed
Closed
Broken tests with "legacy" sys.float_repr_style#128051
Assignees
skirpichev
Labels
testsTests in the Lib/test dirtype-bugAn unexpected behavior, bug, or error

Description

@skirpichev
skirpichev
opened on Dec 18, 2024

Bug report

Bug description:

In #128005 broken (by downstream patch) build reveal some problems in the CPython test suite. Except for few explicitly marked tests, other don't care about different repr() behavior for floats. Many tests fail when interpreter built with _PY_SHORT_FLOAT_REPR set to 0: #128005 (comment)

Currently we don't test platforms with sys.float_repr_style == "legacy". Maybe we should? At least this define can be explicitly set by some configure option and we could test such build.

Then failed tests either should be fixed (by using floats, having exact decimal representation, like 1.25). Or just xfailed out with suitable decorator.

Is this worth?

BTW, as mentioned by Victor Stinner, all PEP 11 platforms support the floatting point "short" format.

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

Activity

  1. skirpichev
    added
    type-bugAn unexpected behavior, bug, or error
    testsTests in the Lib/test dir
    on Dec 18, 2024

  2. vstinner commented on Dec 18, 2024

    @vstinner
    vstinner
    on Dec 18, 2024
    Member

    If something is done, I suggest skipping failing tests if sys.float_repr_style is "legacy".

  3. skirpichev commented on Dec 18, 2024

    @skirpichev
    skirpichev
    on Dec 18, 2024
    MemberAuthor

    There are failing doctests too.

    I think that in most cases it's possible to choose different values. If so, why not fix tests?

    Do you think that there should be a real system with "legacy" repr or it's better just build CPython with -D_PY_SHORT_FLOAT_REPR=0 added to CFLAGS?

    CC @pablogsal (buildbots)

  4. vstinner commented on Dec 19, 2024

    @vstinner
    vstinner
    on Dec 19, 2024
    Member

    We don't test every possible config options in buildbots. For example, I don't think that we have a buildot treating warnings as errors, whereas it would be important to have one (IMO).

  5. pablogsal commented on Jan 21, 2025

    @pablogsal
    pablogsal
    on Jan 21, 2025
    Member

    Do you think that there should be a real system with "legacy" repr or it's better just build CPython with -D_PY_SHORT_FLOAT_REPR=0 added to CFLAGS?

    CC @pablogsal (buildbots)

    Currently we don't have such buldbots unfortunately. Someone interested in testing in this configuration should add one following the procedure

  6. skirpichev
    self-assigned this
    on Mar 3, 2025

  7. skirpichev commented on Jun 20, 2025

    @skirpichev
    skirpichev
    on Jun 20, 2025
    MemberAuthor

    Maybe just add a job in the GitHub CI, that will build CPython with -D_PY_SHORT_FLOAT_REPR=0?

    I have no (alive) hardware around with legacy repr. Though - if we actually support this configuration - I think it's good to test it. E.g. this code is actually changed from time to time. Maybe this also help people to choose better floating-point values in tests (exact or having same repr in legacy/short modes).

    Or... Maybe it's time just to drop 'legacy' repr? Are there alive users of such configuration? CC @mdickinson

    Attached test log with broken tests (./python -m test), around ten of them. Attached patch fix most of tests, few disabled for 'legacy' repr. (Though, maybe I should instead disable more tests, where we can't/won't use some exactly representable decimal fractions.)

    Let me know if this worth a PR.

    legacy.log.txt

    0001-gh-128051-fix-tests-if-sys.float_repr_style-is-legac.patch.txt

  8. mdickinson commented on Jun 20, 2025

    @mdickinson
    mdickinson
    on Jun 20, 2025
    Member

    Or... Maybe it's time just to drop 'legacy' repr? Are there alive users of such configuration? CC @mdickinson

    I'd be delighted to see the "legacy" mode finally die.

    The tricky part is that the "short" repr code requires a way to set the FPU precision, and the mechanism for doing so isn't standardised, so without the legacy mode, getting Python to run on any new platform potentially requires figuring out the FPU precision puzzle for that platform.

    FWIW, I think the "right" solution here is to ditch dtoa.c altogether in favour of something like Dragonbox (for the float-to-string direction) and ... um ... something else for string-to-float conversions. (That something else can't just be strtod, though, since we can't rely on that being correctly rounded.)

  9. mdickinson commented on Jun 20, 2025

    @mdickinson
    mdickinson
    on Jun 20, 2025
    Member

    It's also worth noting that none of the recent crop of improved float-to-string converters (Ryu, Grisu, Dragonbox, ...) replaces even just the float -> string part of dtoa.c completely, since we still want things like this to work:

    >>> import math
>>> f"{math.pi:.53f}"
'3.14159265358979311599796346854418516159057617187500000'

  10. skirpichev commented on Jun 21, 2025

    @skirpichev
    skirpichev
    on Jun 21, 2025
    MemberAuthor

    That something else can't just be strtod, though, since we can't rely on that being correctly rounded.

    The C standard recommends it, if string has at most DBL_DECIMAL_DIG digits. Though, quick tests on Glibc show different timings (here PyOS_ascii_strtod is the current fallback code):

    Benchmark ref PyOS_ascii_strtod strtod
    float('1.25') 306 ns 690 ns: 2.25x slower 561 ns: 1.83x slower
    float('3.141592653589793') 1.06 us 867 ns: 1.22x faster 745 ns: 1.42x faster
    float('1.11231e-10') 334 ns 852 ns: 2.55x slower 732 ns: 2.19x slower
    float('-1.2981e+100') 931 ns 763 ns: 1.22x faster 645 ns: 1.44x faster
    Geometric mean (ref) 1.40x slower 1.18x slower

    since we still want things like this to work:

    >>> import math
>>> f"{math.pi:.53f}"
'3.14159265358979311599796346854418516159057617187500000'

    This is something working with the printf() or I miss something?

    $ cat a.c
#include <stdio.h>
int main()
{
    printf("%.53f\n", 3.141592653589793);
    return 0;
}
$ cc a.c && ./a.out
3.14159265358979311599796346854418516159057617187500000

  11. vstinner commented on Jun 24, 2025

    @vstinner
    vstinner
    on Jun 24, 2025
    Member

    0001-gh-128051-fix-tests-if-sys.float_repr_style-is-legac.patch.txt

    This change mostly LGTM. Please create a PR.

    Attached patch fix most of tests

    It's ok if we don't fix all tests. I don't think that we can say that legacy float repr is currently supported. We provide best effort support.

  12. skirpichev
    added a commit that references this issue on Jun 25, 2025

    pythongh-128051: fix tests if sys.float_repr_style is 'legacy'

    33f984f

  13. skirpichev commented on Jun 25, 2025

    @skirpichev
    skirpichev
    on Jun 25, 2025
    MemberAuthor

    Please create a PR.

    See #135908

    Though, I'm not sure if this change does make sense without new CI job.

  14. vstinner
    added a commit that references this issue on Jun 27, 2025

    gh-128051: Fix tests if sys.float_repr_style is 'legacy' (#135908)

    f3aec60
  15. miss-islington
    added a commit that references this issue on Jun 27, 2025

    pythongh-128051: Fix tests if sys.float_repr_style is 'legacy' (pytho…

    6cc556b
  16. vstinner
    closed this as completedon Jun 27, 2025
  17. vstinner
    added 3 commits that reference this issue on Jun 27, 2025

    pythongh-128051: Fix tests if sys.float_repr_style is 'legacy' (pytho…

    8e7d6f2

    [3.14] gh-128051: Fix tests if sys.float_repr_style is 'legacy' (GH-1…

    33c83ce

    [3.13] gh-128051: Fix tests if sys.float_repr_style is 'legacy' (#135908

    b5fafc3
  18. AndPuQing
    added a commit that references this issue on Jul 11, 2025

    pythongh-128051: Fix tests if sys.float_repr_style is 'legacy' (pytho…

    551f927
  19. Pranjal095
    added a commit that references this issue on Jul 12, 2025

    pythongh-128051: Fix tests if sys.float_repr_style is 'legacy' (pytho…

    0b3f665
  20. taegyunkim
    added a commit that references this issue on Aug 4, 2025

    pythongh-128051: Fix tests if sys.float_repr_style is 'legacy' (pytho…

    2585b08
  21. Agent-Hellboy
    added a commit that references this issue on Aug 19, 2025

    pythongh-128051: Fix tests if sys.float_repr_style is 'legacy' (pytho…

    83bcd76
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

testsTests in the Lib/test dirtype-bugAn unexpected behavior, bug, or error

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions