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

Remove the float.__set_format__() method #91008

Closed
vstinner opened this issue Feb 25, 2022 · 17 comments
Closed

Remove the float.__set_format__() method #91008

vstinner opened this issue Feb 25, 2022 · 17 comments
Labels
3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@vstinner
Copy link
Member

BPO 46852
Nosy @tim-one, @rhettinger, @mdickinson, @vstinner
PRs
  • bpo-46852: Rename float.__set_format__() to float.__setformat__() #31558
  • [3.10] bpo-46852: Rename float.__set_format__() to float.__setformat__() (GH-31558) #31578
  • [3.9] bpo-46852: Rename float.__set_format__() to float.__setformat__() (GH-31558) #31581
  • bpo-46852: Remove the float.__set_format__() method #31585
  • bpo-46852: Building Python requires IEEE 754 support #31592
  • bpo-46852: Restore test_getformat() test #31601
  • 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 2022-03-03.23:39:31.420>
    created_at = <Date 2022-02-25.00:07:57.345>
    labels = ['interpreter-core', '3.11']
    title = 'Remove the float.__set_format__() method'
    updated_at = <Date 2022-03-03.23:39:31.419>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2022-03-03.23:39:31.419>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2022-03-03.23:39:31.420>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2022-02-25.00:07:57.345>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 46852
    keywords = ['patch']
    message_count = 17.0
    messages = ['413943', '413946', '413947', '413948', '413949', '413950', '413953', '413997', '414005', '414011', '414018', '414050', '414131', '414150', '414152', '414158', '414493']
    nosy_count = 4.0
    nosy_names = ['tim.peters', 'rhettinger', 'mark.dickinson', 'vstinner']
    pr_nums = ['31558', '31578', '31581', '31585', '31592', '31601']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue46852'
    versions = ['Python 3.11']

    @vstinner
    Copy link
    Member Author

    It has been decided to require IEEE 754 to build Python 3.11:
    https://mail.python.org/archives/list/python-dev@python.org/thread/J5FSP6J4EITPY5C2UJI7HSL2GQCTCUWN/

    At Python startup, _PyFloat_InitState() checks the IEEE 754 format at runtime. It can be changed using float.__get_format__() and float.__set_format__() methods.

    These methods docstrings say that they only exist to test Python itself:

    "You probably don't want to use this function. It exists mainly to be used in Python's test suite."

    These methods are private and not documented.

    I propose to remove them.

    Once they will be removed, it will become possible to move the detection of the IEEE 754 format in the build step (./configure script) rather than doing the detection at runtime (slower). It would remove an "if" in _PyFloat_Pack4() and _PyFloat_Pack8(), and allow to specialize these functions for the detected format at build time. These functions are used by serialization formats: marshal, pickle and struct.

    @vstinner vstinner added 3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Feb 25, 2022
    @rhettinger
    Copy link
    Contributor

    I would not miss these methods. Unless Mark says they are needed, +1 for removal.

    @vstinner
    Copy link
    Member Author

    See also bpo-46656: "Remove the Py_NO_NAN macro: require NAN to build Python 3.11".

    @vstinner
    Copy link
    Member Author

    Oh wait, I'm now confused by the method names. In Python 3.10, the correct names at:

    • float.__getformat__() <= 4 underscores
    • float.__set_format__() <= 5 underscores

    It's even more confusing because the "set format" is only used in one place: test_float, and test_float uses... __setformat__() (4 underscores).

    A typo a was introduced in Python 3.7 by:

    commit b5c51d3
    Author: Serhiy Storchaka <storchaka@gmail.com>
    Date: Sat Mar 11 09:21:05 2017 +0200

    bpo-20185: Convert float object implementation to Argument Clinic. (#543)
    
    Based on patch by Vajrasky Kok.
    

    Since Python 3.7, the 4 "set format" tests are simply skipped!

    $ ./python -m test -v test_float
    (...)
    test_getformat (test.test_float.FormatFunctionsTestCase) ... skipped 'requires __setformat__'
    test_setformat (test.test_float.FormatFunctionsTestCase) ... skipped 'requires __setformat__'
    (...)
    test_double_specials_dont_unpack (test.test_float.UnknownFormatTestCase) ... skipped 'requires __setformat__'
    test_float_specials_dont_unpack (test.test_float.UnknownFormatTestCase) ... skipped 'requires __setformat__'
    (...)

    Moreover, unittest.mock supports mocking __setformat__() (4 underscores).

    @vstinner
    Copy link
    Member Author

    I wrote #75739 to fix the typo in the method name.

    @vstinner
    Copy link
    Member Author

    We can keep the float.__getformat__() method, it doesn't harm. I change the issue title to only propose to remove the float.__setformat__() method.

    @vstinner vstinner changed the title Remove float.__get_format__() and float.__set_format__() Remove the float.__setformat__() method Feb 25, 2022
    @vstinner vstinner changed the title Remove float.__get_format__() and float.__set_format__() Remove the float.__setformat__() method Feb 25, 2022
    @vstinner
    Copy link
    Member Author

    New changeset 7d03c8b by Victor Stinner in branch 'main':
    bpo-46852: Rename float.__set_format__() to float.__setformat__() (GH-31558)
    7d03c8b

    @vstinner
    Copy link
    Member Author

    New changeset 0848da1 by Victor Stinner in branch '3.10':
    bpo-46852: Rename float.__set_format__() to float.__setformat__() (GH-31558) (GH-31578)
    0848da1

    @vstinner
    Copy link
    Member Author

    New changeset a549cd1 by Victor Stinner in branch '3.9':
    bpo-46852: Rename float.__set_format__() to float.__setformat__() (GH-31558) (GH-31581)
    a549cd1

    @mdickinson
    Copy link
    Member

    I'd be happy to see float.__setformat__ go, if it's not still needed for Python's test suite (which was its entire raison d'être). If no-one noticed the accidental misnaming, then it's pretty clear no-one's been using it.

    I'd like to bet that there are at least a few people out there using float.__getformat__, despite that its docstring says "You probably don't want to use this function".

    Maybe we could consider moving the information contained in __getformat__ to somewhere more accessible (e.g., a new field in sys.float_info)?

    @vstinner
    Copy link
    Member Author

    Mark Dickinson:

    I'd be happy to see float.__setformat__ go, if it's not still needed for Python's test suite (which was its entire raison d'être). If no-one noticed the accidental misnaming, then it's pretty clear no-one's been using it.

    Nobody noticed the since Python 3.7 (released in June 2018). Well, even test_float didn't use it :-D (I just fixed the typo yesterday.) So I expect that no one uses it.

    I'd like to bet that there are at least a few people out there using float.__getformat__, despite that its docstring says "You probably don't want to use this function".

    Yeah, I changed my mind and I prefer to leave it unchanged for now. It doesn't prevent me to optimize _PyFloat_Pack8().

    @vstinner
    Copy link
    Member Author

    New changeset 5ab745f by Victor Stinner in branch 'main':
    bpo-46852: Remove the float.__set_format__() method (GH-31585)
    5ab745f

    @vstinner
    Copy link
    Member Author

    New changeset 5a1c637 by Victor Stinner in branch 'main':
    bpo-46852: Restore test_getformat() test (GH-31601)
    5a1c637

    @mdickinson
    Copy link
    Member

    Thanks, Victor. I think this can be closed now.

    @vstinner
    Copy link
    Member Author

    I reopen the issue for the second part of my plan:

    "Once they will be removed, it will become possible to move the detection of the IEEE 754 format in the build step (./configure script) rather than doing the detection at runtime (slower). It would remove an "if" in _PyFloat_Pack4() and _PyFloat_Pack8(), and allow to specialize these functions for the detected format at build time. These functions are used by serialization formats: marshal, pickle and struct."

    @vstinner vstinner reopened this Feb 27, 2022
    @vstinner vstinner reopened this Feb 27, 2022
    @mdickinson
    Copy link
    Member

    I reopen the issue for the second part of my plan

    Hmm. That sounds like it should be a separate issue, or at the least, this issue should be retitled. It's helpful to keep issue titles accurate.

    @vstinner
    Copy link
    Member Author

    vstinner commented Mar 3, 2022

    I created a follow-up issue: bpo-46917 "Require IEEE 754 floating point to build Python 3.11". I close this one: float.__set_format__() has been removed.

    @vstinner vstinner closed this as completed Mar 3, 2022
    @vstinner vstinner changed the title Remove the float.__setformat__() method Remove the float.__set_format__() method Mar 3, 2022
    @vstinner vstinner closed this as completed Mar 3, 2022
    @vstinner vstinner changed the title Remove the float.__setformat__() method Remove the float.__set_format__() method Mar 3, 2022
    @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.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants