Skip to content

Wrong PEP 515 parsing in decimal module (both C and Python versions) #88433

Closed
Closed
Wrong PEP 515 parsing in decimal module (both C and Python versions)#88433
Labels
3.11only security fixesextension-modulesC modules in the Modules dirstdlibStandard Library Python modules in the Lib/ directory

Description

@skirpichev
skirpichev
mannequin
opened on May 30, 2021
BPO 44267
Nosy @rhettinger, @facundobatista, @mdickinson, @skirpichev
Files
  • _pydecimal-pep515.diff
  • 0001-bpo-44267-fix-parsing-Decimal-s-with-underscores.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 = None
closed_at = <Date 2021-05-30.09:05:38.324>
created_at = <Date 2021-05-30.04:12:10.330>
labels = ['extension-modules', 'library', '3.11']
title = 'Wrong PEP 515 parsing in decimal module (both C and Python versions)'
updated_at = <Date 2021-05-30.09:20:39.447>
user = 'https://github.com/skirpichev'

    bugs.python.org fields:

    activity = <Date 2021-05-30.09:20:39.447>
actor = 'Sergey.Kirpichev'
assignee = 'none'
closed = True
closed_date = <Date 2021-05-30.09:05:38.324>
closer = 'mark.dickinson'
components = ['Extension Modules', 'Library (Lib)']
creation = <Date 2021-05-30.04:12:10.330>
creator = 'Sergey.Kirpichev'
dependencies = []
files = ['50071', '50072']
hgrepos = []
issue_num = 44267
keywords = ['patch']
message_count = 8.0
messages = ['394750', '394755', '394756', '394757', '394758', '394759', '394760', '394761']
nosy_count = 4.0
nosy_names = ['rhettinger', 'facundobatista', 'mark.dickinson', 'Sergey.Kirpichev']
pr_nums = []
priority = 'normal'
resolution = 'wont fix'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue44267'
versions = ['Python 3.11']

    Activity

    1. skirpichev commented on May 30, 2021

      @skirpichev
      skirpichevmannequin
      on May 30, 2021
      MannequinAuthor

      While working on bpo-44258 I discover that the decimal module doesn't follow specification in PEP-515: "The current proposal is to allow one underscore between digits, and after base specifiers in numeric literals." (c)

      For example:
>>> float("1.1__1")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: could not convert string to float: '1.1__1'

but
>>> from decimal import Decimal as C
>>> from _pydecimal import Decimal as P
>>> C("1.1__1")
Decimal('1.11')
>>> P("1.1__1")
Decimal('1.11')

      Maybe this requirement could be relaxed in PEP, but it seems - this was already discussed (see Alternative Syntax section). Hence, I think this is a bug.

      Patch for _pydecimal attached.

    2. skirpichev
      added
      3.11only security fixes
      extension-modulesC modules in the Modules dir
      stdlibStandard Library Python modules in the Lib/ directory
      on May 30, 2021

    3. mdickinson commented on May 30, 2021

      @mdickinson
      mdickinson
      on May 30, 2021
      Member

      There was some discussion of this on the python-dev mailing list at the time - see https://mail.python.org/pipermail/python-dev/2016-March/143556.html and the surrounding thread. It's probably best left alone.

    4. skirpichev commented on May 30, 2021

      @skirpichev
      skirpichevmannequin
      on May 30, 2021
      MannequinAuthor

      On Sun, May 30, 2021 at 08:20:14AM +0000, Mark Dickinson wrote:

      There was some discussion of this on the python-dev mailing list at the time

      I see.

      It's probably best left alone.

      PEP-515 is clear. If this is not a bug - it should be adjusted (as it
      claims to cover Decimal's among other stuff).

    5. mdickinson commented on May 30, 2021

      @mdickinson
      mdickinson
      on May 30, 2021
      Member

      If this is not a bug - it should be adjusted

      Standards Track PEPs are historical documents; it's quite common that an actual implementation ends up diverging from a PEP in small ways after the PEP is accepted, and we don't usually do post-hoc updates in those situations.

      Possibly the decimal documentation could be updated, though.

    6. skirpichev commented on May 30, 2021

      @skirpichev
      skirpichevmannequin
      on May 30, 2021
      MannequinAuthor

      On Sun, May 30, 2021 at 08:32:40AM +0000, Mark Dickinson wrote:

      Standards Track PEPs are historical documents; it's quite common that an
      actual implementation ends up diverging from a PEP in small ways after
      the PEP is accepted, and we don't usually do post-hoc updates in those situations.

      Well, then I something misunderstood in PEP-0:
      --->8------
      If changes based on implementation experience and user feedback are made
      to Standards track PEPs while in the Accepted or Provisional State,
      those changes should be noted in the PEP, such that the PEP accurately
      describes the state of the implementation at the point where it is
      marked Final.
      ---->8--------
      I don't think that PEP describes the state of art in the decimal module.

      Possibly the decimal documentation could be updated, though.

      The current behaviour is documented. Do you mean we should document
      disagreement with PEP as well?

      Regarding mail thread: I don't think that following the PEP will
      slow down string conversion. Also, probably we want that strings
      that supported by float() and Decimal() were interchangeable.

    7. mdickinson commented on May 30, 2021

      @mdickinson
      mdickinson
      on May 30, 2021
      Member

      Well, then I something misunderstood in PEP-0

      Yep, you're absolutely right. I should have said "after the PEP is final", not "after the PEP is accepted". PEP-515 was marked final on April 28th, 2017.

      The current behaviour is documented.

      Thanks; I missed that. In that case, I don't think there's anything to do here documentation-wise.

    8. mdickinson commented on May 30, 2021

      @mdickinson
      mdickinson
      on May 30, 2021
      Member

      Regarding mail thread: I don't think that following the PEP will
      slow down string conversion.

      Sorry, I just don't think it's worth re-opening this discussion; Stefan Krah had good reasons (not just speed) to avoid implementing a precise interpretation of PEP-515 for Decimal.

      It would also be a backwards incompatible change at this point to start refusing strings that were previously accepted. As I said, it's probably best left alone at this point.

    9. mdickinson
      closed this as completedon May 30, 2021
    10. mdickinson
      closed this as completedon May 30, 2021

    11. skirpichev commented on May 30, 2021

      @skirpichev
      skirpichevmannequin
      on May 30, 2021
      MannequinAuthor

      On Sun, May 30, 2021 at 08:58:56AM +0000, Mark Dickinson wrote:

      Yep, you're absolutely right. I should have said "after the PEP is final"

      Unfortunately, neither correction can fix that the PEP does not
      "accurately describes the state of the implementation at the point where
      it is marked Final."

      It would also be a backwards incompatible change at this point to
      start refusing strings that were previously accepted.

      I'm not sure...

      Well, it's not so clear which strings are accepted previously (i.e.
      what's was documented). PEP-515 claims one. The docs says something
      different:
      ---->8---
      If value is a string, it should conform to the decimal numeric string
      syntax after leading and trailing whitespace characters, as well as
      underscores throughout, are removed
      ---->8-------

      and
      --->8------
      Underscores are allowed for grouping, _as with integral and
      floating-point literals in code_.
      -->8-------

      The 1-st sentence doesn't specify the way underscores are removed. And
      given the 2-nd sentence: it's clearly can't be like the current
      behaviour of the Decimal constructor.

    12. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    13. skirpichev
      marked Decimal class allows underscores that are not PEP 515 compliant #121382 as a duplicate of this issue on Oct 19, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.11only security fixesextension-modulesC modules in the Modules dirstdlibStandard Library Python modules in the Lib/ directory

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions