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 constructor accept all unicode decimal digits in input. #50844

Closed
mdickinson opened this issue Jul 29, 2009 · 9 comments
Closed
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@mdickinson
Copy link
Member

BPO 6595
Nosy @rhettinger, @mdickinson, @ericvsmith, @ezio-melotti
Files
  • issue6595.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-08-02.11:03:41.728>
    created_at = <Date 2009-07-29.08:39:53.500>
    labels = ['type-feature', 'library']
    title = 'Make Decimal constructor accept all unicode decimal digits in input.'
    updated_at = <Date 2009-08-02.11:03:41.727>
    user = 'https://github.com/mdickinson'

    bugs.python.org fields:

    activity = <Date 2009-08-02.11:03:41.727>
    actor = 'mark.dickinson'
    assignee = 'mark.dickinson'
    closed = True
    closed_date = <Date 2009-08-02.11:03:41.728>
    closer = 'mark.dickinson'
    components = ['Library (Lib)']
    creation = <Date 2009-07-29.08:39:53.500>
    creator = 'mark.dickinson'
    dependencies = []
    files = ['14593']
    hgrepos = []
    issue_num = 6595
    keywords = ['patch']
    message_count = 9.0
    messages = ['91030', '91031', '91032', '91033', '91051', '91092', '91133', '91180', '91181']
    nosy_count = 4.0
    nosy_names = ['rhettinger', 'mark.dickinson', 'eric.smith', 'ezio.melotti']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue6595'
    versions = ['Python 2.6', 'Python 3.1', 'Python 2.7', 'Python 3.2']

    @mdickinson
    Copy link
    Member Author

    Ezio Melotti asked (on #python-dev) why the Decimal constructor doesn't
    accept decimal digits other than 0-9. As far as I can tell there's no
    good reason for it not to. Moreover, the standard on which the decimal
    module is based says[1]:

    """It is recommended that implementations also provide additional number
    formatting routines (including some which are locale-dependent), and if
    available should accept non-European decimal digits in strings."""

    All other builtin or standard library numeric types already accept such
    digits:

    Python 3.2a0 (py3k:74247, Jul 29 2009, 09:28:12) 
    [GCC 4.0.1 (Apple Inc. build 5493)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> from fractions import Fraction
    >>> from decimal import Decimal
    >>> x = '\uff11\uff10\uff15\uff18\uff15'
    >>> x
    '10585'
    >>> int(x)
    10585
    >>> float(x)
    10585.0
    >>> complex(x)
    (10585+0j)
    >>> Fraction(x)
    Fraction(10585, 1)
    >>> Decimal(x)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/Users/dickinsm/python/svn/py3k/Lib/decimal.py", line 548, in 
    __new__
        "Invalid literal for Decimal: %r" % value)
      File "/Users/dickinsm/python/svn/py3k/Lib/decimal.py", line 3816, in 
    _raise_error
        raise error(explanation)
    decimal.InvalidOperation: Invalid literal for Decimal: '10585'

    I propose adding support for this in Python 3.2 and (possibly) 2.7. The
    change would be for input only: no record of the original form of the
    digits would be kept by the Decimal object itself, so that e.g.,
    str(Decimal('10585')) would still be '10585'.

    [1] See http://speleotrove.com/decimal/daconvs.html

    @mdickinson mdickinson self-assigned this Jul 29, 2009
    @mdickinson mdickinson added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jul 29, 2009
    @mdickinson
    Copy link
    Member Author

    Here's a patch

    @ericvsmith
    Copy link
    Member

    +1

    The standard recommends it, and the other numeric types support it, so
    Decimal should as well.

    @ericvsmith
    Copy link
    Member

    Since you're calling int() on the result, can't this code:
    self._int = str(int((intpart+fracpart).lstrip('0') or '0'))
    just be:
    self._int = str(int(intpart+fracpart))
    ?

    And here, you already know diag is not None, so do you need the "or '0'"
    part?
    self._int = str(int(diag or '0')).lstrip('0')

    And, in both calls to .lstrip('0'), what happens if you have a
    non-European leading '0', like '\uff10'?

    Otherwise, the patch looks good to me.

    @rhettinger
    Copy link
    Contributor

    +1

    Also, I would like to see this backported. We've long promised that any
    variance with the spec will be treated as a bugfix. The change won't
    break any existing code.

    @mdickinson
    Copy link
    Member Author

    Thanks for the feedback; I've added 2.6, 2.7, 3.1 to the
    versions and will backport.

    [Eric]

    Since you're calling int() on the result, can't this code:
    self._int = str(int((intpart+fracpart).lstrip('0') or '0'))
    just be:
    self._int = str(int(intpart+fracpart))
    ?

    Yes! Thank you. I'm not sure what (if anything) I was thinking here.

    The str(int(...)) hack is quite an ugly way to normalize a string; it
    also has the drawback of taking time quadratic in the length of the
    input. In its defence: (a) I can't think of anything better; (b) this
    change seems to have no noticeable effect on the time to run the test-
    suite, and (c) since all the arithmetic operations use string <-> int
    conversions anyway the quadratic time behaviour doesn't really make
    things any worse than they already are.

    And here, you already know diag is not None, so do you need the "or
    '0'"
    part?
    self._int = str(int(diag or '0')).lstrip('0')

    I think *something* like this is needed, since diag can legitimately be
    the empty string. It might be clearer as a proper if block, though:
    'if diag: ... else: ...'.

    @rhettinger
    Copy link
    Contributor

    I like the str(int(...)) approach because it guarantees handling that
    is consistent with other types.

    @mdickinson
    Copy link
    Member Author

    Committed to py3k in r74279, release31-maint in r74280. Leaving open for
    backport to 2.x.

    @mdickinson
    Copy link
    Member Author

    Backported to trunk and release26-maint in r74281 and r74282.

    @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-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants