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

Decimal to receive from_float method #49046

Closed
stevendaprano opened this issue Dec 31, 2008 · 20 comments
Closed

Decimal to receive from_float method #49046

stevendaprano opened this issue Dec 31, 2008 · 20 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@stevendaprano
Copy link
Member

BPO 4796
Nosy @rhettinger, @facundobatista, @mdickinson, @stevendaprano
Files
  • decimal2.diff: Add from_float() methods
  • 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/rhettinger'
    closed_at = <Date 2009-01-03.19:21:03.375>
    created_at = <Date 2008-12-31.22:59:19.000>
    labels = ['type-feature', 'library']
    title = 'Decimal to receive from_float method'
    updated_at = <Date 2009-01-11.05:03:48.452>
    user = 'https://github.com/stevendaprano'

    bugs.python.org fields:

    activity = <Date 2009-01-11.05:03:48.452>
    actor = 'rhettinger'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2009-01-03.19:21:03.375>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2008-12-31.22:59:19.000>
    creator = 'steven.daprano'
    dependencies = []
    files = ['12562']
    hgrepos = []
    issue_num = 4796
    keywords = ['needs review']
    message_count = 20.0
    messages = ['78664', '78670', '78671', '78781', '78784', '78789', '78793', '78796', '78843', '78846', '78875', '78931', '78942', '78946', '78993', '79045', '79084', '79579', '79580', '79588']
    nosy_count = 4.0
    nosy_names = ['rhettinger', 'facundobatista', 'mark.dickinson', 'steven.daprano']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue4796'
    versions = ['Python 3.1', 'Python 2.7']

    @stevendaprano
    Copy link
    Member Author

    In the PEP for Decimal, it was discussed that the class should have a
    from_float() method for converting from floats, but to leave it out of
    the Python 2.4 version:
    http://www.python.org/dev/peps/pep-0327/#from-float

    Following discussions with Mark Dickinson, I would like to request
    that this now be implemented.

    The suggested API is:
    Decimal.from_float(floatNumber, [decimal_places])

    At the risk of derailing the request, I wonder whether it is better to
    give a context rather than a number of decimal places?

    Pro: better control over conversion, as you can specify rounding
    method as well as number of decimal places.

    Con: more difficult for newbies to understand.

    Semi-pro: float to decimal conversions are inherently tricky, perhaps
    we should be discouraging newbies from blindly calling from_float()
    and make the (hypothetical) context argument mandatory rather than
    optional?

    @stevendaprano stevendaprano added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Dec 31, 2008
    @rhettinger rhettinger self-assigned this Dec 31, 2008
    @rhettinger
    Copy link
    Contributor

    The decimal constructor should be lossless. The underlying spec was
    designed with the notion that all numbers in decimal are exact;
    operations can be lossy but the numbers themselves are exact.
    Accordingly, I recommend Decimal.from_float(f) with no qualifiers or
    optional arguments.

    To support the use case of wanting to round the input, I suggest a
    separate method modeled on Context.create_decimal(). It can either be
    an extension of the existing method or a new method like
    Context.create_decimal_from_float(). I recommend the former since
    rounding is already implied by the context qualifier. Either way, the
    effect would be the same as Decimal.from_float(f) + 0 in a given
    context. Per the docs: "Creates a new Decimal instance from num but
    using self as context. Unlike the Decimal constructor, the context
    precision, rounding method, flags, and traps are applied to the conversion."

    @rhettinger
    Copy link
    Contributor

    FYI, there is already a lossless implementation in the docs:

    def float_to_decimal(f):
        "Convert a floating point number to a Decimal with no loss of
    information"
        n, d = f.as_integer_ratio()
        with localcontext() as ctx:
            ctx.traps[Inexact] = True
            while True:
                try:
                   return Decimal(n) / Decimal(d)
                except Inexact:
                    ctx.prec += 1

    @mdickinson
    Copy link
    Member

    Accordingly, I recommend Decimal.from_float(f) with no qualifiers or
    optional arguments.

    I agree with this. Since lossless conversion is feasible, this seems
    the obvious way to go.

    It's lucky that the exponent range for (binary) floats is relatively
    small, though: for IEEE 754 floats the worst case conversions (e.g.,
    (2**53-1)/2**1074) produce Decimals with something like 767 significant
    digits, which is still plenty small enough not to cause difficulties.

    The recipe in the docs is not industrial strength: it doesn't handle
    infinities, nans and negative zero. I think there's a place for a
    from_float method that handles these special cases correctly---or at
    least as correctly as reasonable: +-infinity should convert to +-
    infinity, nans to (quiet) nans. I don't think it's worth worrying about
    preserving the sign or payload of a binary nan: just convert all float
    nans to Decimal('NaN'). I do however think it's worth trying to
    preserve the sign of negative zero. Note: I'm not suggesting changing
    the *documentation* at all---the recipe there is fine as it is, IMO, but
    I think an official version should include these corner cases.

    @mdickinson
    Copy link
    Member

    One also has to worry about the exponent of the converted result: e.g.,
    should Decimal.from_float(10.0) produce Decimal('1E1') or Decimal('10')?
    The latter looks nicer, to me.

    IEEE 754 isn't much help here: as far as I can tell it says nothing about
    binary <-> decimal conversions.

    I see two reasonable strategies: (1) always use the largest exponent
    possible (so we'd get Decimal('1E1') above), or (2) when the quantity
    converted is an exact integer, use an exponent of zero; otherwise fall
    back to (1).

    Option (2) is pretty much what the recipe in the docs does already, I
    think: it computes a quotient of two Decimals, each having exponent zero,
    so the preferred exponent of the result is also zero.

    @stevendaprano
    Copy link
    Member Author

    Mark suggested the following strategy for Decimal.from_float: "always
    use the largest exponent possible".

    Just for the avoidance of all doubt, do you mean the largest exponent
    with the number normalised to one digit to the right of the decimal
    place? Because 1e1 = 0.1e2 = 0.01e3 = ... and there is no "largest
    exponent possible" if you allow unnormalised numbers. Seems obvious to
    me, but maybe I'm missing something.

    @stevendaprano
    Copy link
    Member Author

    Raymond:

    Accordingly, I recommend Decimal.from_float(f) with no
    qualifiers or optional arguments.

    -0 on this one. It's going to confuse an awful lot of newbies when
    they write Decimal.from_float(1.1) and get
    Decimal('110000000000000008881784197001252...e-51').

    Also, why not just extend the Decimal() constructor to accept a float
    as the argument? Why have a separate from_float() method at all?

    To support the use case of wanting to round the input, I
    suggest a separate method modeled on Context.create_decimal().

    +1 on this.

    @mdickinson
    Copy link
    Member

    Just for the avoidance of all doubt, do you mean the largest exponent
    with the number normalised to one digit to the right of the decimal
    place?

    No. I'm using 'exponent' in the sense described in the standard. See:

    http://speleotrove.com/decimal/dbover.html

    Equivalently, it's the value of the _exp attribute for a Decimal
    instance. (For the purposes of disambiguation, the alternative exponent
    that you describe above is often referred to as the 'adjusted exponent'
    in the documentation and code.)

    Briefly, every finite Decimal can be thought of as a triple (sign,
    coefficient, exponent), representing the value (-1)**sign * coefficient

    • 10**exponent, with the coefficient an integer. It's this exponent
      that should be maximized.

    Because 1e1 = 0.1e2 = 0.01e3 = ... and there is no "largest
    exponent possible"

    All these have exponent 1:

    >>> Decimal('1e1')._exp
    1
    >>> Decimal('0.1e2')._exp
    1
    >>> Decimal('0.01e3')._exp
    1

    IOW, leading zeros have no significance; only trailing zeros do.

    Also, why not just extend the Decimal() constructor to accept a float
    as the argument? Why have a separate from_float() method at all?

    This was discussed extensively when the decimal module was being
    proposed; see the Decimal PEP for arguments against this.

    @stevendaprano
    Copy link
    Member Author

    Mark wrote:

    > Also, why not just extend the Decimal() constructor to accept
    > a float as the argument? Why have a separate from_float()
    > method at all?

    This was discussed extensively when the decimal module was
    being proposed; see the Decimal PEP for arguments against this.

    I'm very aware of that. The Decimal PEP says the consensus was for
    from_float() to take a second argument specifying the precision.
    Decimal(1.1) => Decimal("1.1") was rejected for the reasons given in
    the PEP by Paul Moore, and Decimal(1.1) =>
    Decimal('110000000000000008881784197001252...e-51') was (presumably)
    rejected because it would confuse newbies. Hence the decision to (1)
    make an alternative constructor and (2) have it take a second
    argument.

    It looks like you and Raymond have rejected #2 but are keeping #1, and
    I'm curious why. That's genuine curiosity, and a real question, not a
    thinly-veiled scowl of disapproval disguised as a question :)

    Anyway, I'm happy enough so long as Raymond's suggested
    Context.create_decimal() exists, that's the actual functionality I'm
    after, so maybe I should let you guys get on with it. Thanks.

    @mdickinson
    Copy link
    Member

    It looks like you and Raymond have rejected #2 but are keeping #1

    I'm not against #2, but I'm not particularly for it either. In any case,
    once you've converted your float to Decimal it's trivial to round it to
    whatever precision you feel like, so #2 seems unnecessary to me. -0.0.

    I am -1.100000000000000088817841970012523233890533447265625 on any
    implementation of from_float (with or without keywords, defaults, etc.)
    for which Decimal.from_float(1.1) gives Decimal('1.1').

    @rhettinger
    Copy link
    Contributor

    once you've converted your float to Decimal it's trivial
    to round it to whatever precision you feel like, so #2
    seems unnecessary to me.

    Agree with Mark on how to control rounding. The DecimalWay(tm) is that
    used by Context.create_decimal(). A second argument is problematic for
    two reasons. First, it demands that some rounding take place but
    doesn't let you control the rounding method or signal an Inexact
    conversion -- you need a context for that. Second, the API for decimal
    is already somewhat complex and we don't want to make it worse by
    introducing a new variant with a second argument that has no parallel
    elsewhere in the API.

    Also agree with Mark regarding Decimal.from_float() which needs to be
    lossless and exact. It parallels fractions.from_float() which does the
    same thing for rationals.

    The DecimalWay(tm) is to have Decimal constructors be exact and to use
    Context based constructors when rounding is desired.

    I'll submit a patch to this effect (unless someone beats me to it).

    @rhettinger
    Copy link
    Contributor

    See attached patch for Py27.

    @rhettinger rhettinger assigned mdickinson and unassigned rhettinger Jan 3, 2009
    @mdickinson
    Copy link
    Member

    Instead of the repeated divisions and Inexact tests, how about a direct
    approach: n/2**k = (n*5**k)/10**k, so something like:

        sign = 0 if copysign(1.0, self) == 1.0 else 1
        n, d = abs(self).as_integer_ratio()
        k = d.bit_length() - 1
        return _dec_from_triple(sign, str(n*5**k), -k)

    should work, and would likely be faster too.

    I also think the sign of 0 should be preserved: i.e.,

    >>> Decimal.from_float(-0.0)
    Decimal('-0')

    Am still reviewing---more comments to come.

    @mdickinson
    Copy link
    Member

    A couple more things:

    1. There's a typo 'equilvalent' in the decimal.py part of the patch.

    2. Can I suggest using

    return d._fix(self)

    instead of

    return self.plus(d)

    in create_decimal_from_float. The plus method does two things: rounds
    to the current context *and* converts -0.0 to 0.0; we only want the
    first of these.

    (It's always been a source of mild irritation to me that there's no
    public method for simply rounding a Decimal to a given context---i.e., a
    public equivalent of _fix.)

    @mdickinson mdickinson assigned rhettinger and unassigned mdickinson Jan 3, 2009
    @rhettinger
    Copy link
    Contributor

    The direct method is *much* faster!
    Applied Mark's suggestions.
    Committed as r68208 and r68211 .

    @mdickinson
    Copy link
    Member

    Raymond,

    Do you think it would be worth replacing the two uses of
    conditional expressions in Decimal.from_float with if-else
    statements?

    Alex Goretoy pointed out (on c.l.p) that the trunk version of
    decimal.py can no longer be imported into Python 2.4 (and 2.3).
    I don't know how much this matters, but it seems to go against
    the comments about 2.3 compatibility at the top of decimal.py.
    I admit that I don't really understand the motivation for these
    comments, or whether they're still relevant 4 versions on
    from Python 2.3.

    Of course, from_float still won't work with earlier versions
    of Python, but having one Decimal method unavailable seems
    like a lesser crime than making 'import decimal' fail.

    @rhettinger
    Copy link
    Contributor

    Do you think it would be worth replacing the two uses of
    conditional expressions in Decimal.from_float with if-else
    statements?

    Yes, please.

    Of course, from_float still won't work with earlier versions
    of Python, but having one Decimal method unavailable seems
    like a lesser crime than making 'import decimal' fail.

    Right. I don't see an easy way around that short of having
    a conditional compilation, allowing use of alternative slow
    code multiplying the float repeatedly by two to build-up
    the float digits.

    @facundobatista
    Copy link
    Member

    Raymond, Mark, thanks for this work!

    I'd include the following in the PEP (under the "from float"
    discussion), what do you think?

    """
    Update: The .from_float() method was added to Python 2.7 and 3.1
    versions, providing lossless and exact conversion from float to Decimal
    (see bpo-4796 [7]_ for further information).

    It has the following syntax::

        Decimal.from_float(floatNumber)

    where floatNumber is the float number origin of the construction.
    Example::

        >>> Decimal.from_float(1.1)
        Decimal('1.100000000000000088817841970012523233890533447265625')
    """

    @mdickinson
    Copy link
    Member

    I agree the PEP should be updated. Your proposed update looks good to me.

    @rhettinger
    Copy link
    Contributor

    Go ahead with the update, but it isn't really necessary. The PEPs are
    for getting something into the language in the first place and for
    centralizing a major discussion. They typically get out of date quickly
    after they've been implemented. In this case, we've got the tracker
    discussion to document the minor discussions than led to this minor
    change in the API. So, it's okay to update the PEP or not.

    @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

    4 participants