This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Decimal accepting Fraction
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: Ramchandra Apte, cvrebert, eric.smith, joncle, mark.dickinson, ncoghlan, rhettinger, skrah, terry.reedy, zach.ware
Priority: low Keywords: patch

Created on 2012-06-22 11:51 by joncle, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue15136_as_decimal.patch zach.ware, 2012-08-23 18:11 Patch to add as_decimal method to fractions.Fraction review
issue15136_with_tests.patch zach.ware, 2012-08-24 21:17 Now with tests! review
fracdec.py terry.reedy, 2012-08-25 18:24 fraction to decimal string
Messages (18)
msg163397 - (view) Author: Jon Clements (joncle) Date: 2012-06-22 11:51
I'm not a numeric expert but I was looking at a post on S/O which related to converting a Fraction to a certain amount of decimal places. I've had a hunt on the tracker but couldn't find anything relevant, but if I've missed it, I apologise.

# F=Fraction, D=Decimal classes

If I try num = D( F(5, 7) )

I get: TypeError: Cannot convert Fraction(5, 7) to Decimal

So I do:

>>> D(f.numerator) / D(f.denominator)
Decimal('0.7142857142857142857142857143')

Which I think is the correct result?

I guess my question is - should Decimal do this implicitly for Fraction?
msg163398 - (view) Author: Ramchandra Apte (Ramchandra Apte) * Date: 2012-06-22 12:14
+1
msg163399 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-06-22 12:32
> I guess my question is - should Decimal do this implicitly for Fraction?

I'd prefer not.  All other cases of Decimal construction (from float, from string, etc.) are lossless with results that don't depend on the current context;  construction from a Fraction will usually involve rounding, with results depending on the current rounding-mode and precision.

Having the division operation explicit (making it obvious that there's rounding involved) looks better to me.
msg163400 - (view) Author: Jon Clements (joncle) Date: 2012-06-22 12:40
Mark - I bow to your superiour knowledge here. However, would not a classmethod of .from_fraction be welcome?

ie, I could write:

d = D.from_fraction(5, 7)

Then the documents labour the point about what you've mentioned?

Just an idea, but fully realise you're the man best suited to decide, so I'll be happy with whatever you say.
msg163401 - (view) Author: Jon Clements (joncle) Date: 2012-06-22 12:45
Not sure what's going on with my machine today: keep sending things to early.

I meant:

D.from_fraction(F)

where if F is not of type Fraction, then the args are used to construct a Fraction - so can use an existing or create one.
msg163402 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-06-22 12:48
> would not a classmethod of .from_fraction be welcome?

That depends. :-)  Certainly a new classmethod seems better to me than extending the constructor to allow Fractions.  I'm not convinced that there's a real need for this feature, though, especially given how easy it is do directly.

So I'm -1 on extending the constructor, -0 on adding a .from_fraction method.

I'm adding other decimally people to the nosy for their opinions.
msg163405 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-06-22 13:34
BTW, the StackOverflow question helped me understand the use-case here:

  http://stackoverflow.com/q/11154954/270986

The perspective is that of a *Fraction* user who wants to be able to easily see the Decimal expansion of a Fraction to an arbitrary number of decimal places.  (I was trying to understand why a *Decimal* user would care about converting from Fraction instances, but that's the wrong way around.)

That desire to have an easy way to see the digits of a Fraction seems reasonable to me, but I'm not sure that having a Decimal method is the right way to go about it.  Another possible solution would be to implement a decent __format__ method for Fraction, so that somebody could do:

>>> format(Fraction(1, 7), '.17g')
>>> '0.14285714285714286'
msg163433 - (view) Author: Jon Clements (joncle) Date: 2012-06-22 16:45
The more I think about this - the shades of grey kick in.

D.from_fraction(F or creatable F) 

Then it would be 'reasonable to assume' for a F.to_decimal() to exist.

Possibly with an optional context argument.

Then, what happens if I do D('2.45') * F(24 / 19)...

I'll leave it to the experts, but have noticed Raymond has ideas for this, but not yet commented?
msg163540 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-06-23 07:30
Something like Fraction.as_decimal(prec=28) would be reasonable.
msg163543 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-06-23 07:36
> Something like Fraction.as_decimal(prec=28) would be reasonable.

I'd prefer an implementation of Fraction.__format__.  That solves the SO user's need exactly.  Note that he/she didn't care about the Decimal type, but only wanted to be able to *print* digits of a Fraction;  the __format__ method is the OOWTDI.
msg163546 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-06-23 08:11
I think *both* proposals are sensible. Fraction already has .from_decimal (using Decimal), so .to_decimal (also using Decimal) is sensible. It also has .from_float, with 'f.to_float' spelled f.__float__, normally called as float(f).

On the other hand, part of the point of the new format system was/is to allow 'other' classes to tie into format specs with custom .__format__. Currently, Fraction inherits .__format__ from object, which only recognizes 's' specifications. (Anything else gives a misleading 'str' error message that is the subject of another issue.) I think it should get a custom .__format__, which could use f.to_decimal(prec), where prec is calculated from the format spec.
msg168954 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2012-08-23 18:11
I came across this issue and thought it might be within my means to attempt a patch, so here's the first step towards one.  I liked Terry's suggestion of implementing __format__ using .as_decimal, so that's what I'm working towards with this.

The first part is (or seems to be, I am pretty new at this and may well have missed something obvious :-)) pretty easy.  The attached patch is pretty self-explanatory, I think. I figure if the user wants something specific from their fraction-as-a-decimal, they're probably working with other decimals already and will be able to supply a context.  Otherwise, if they just want to see a decimal representation, they'll probably be fine with the default context and don't have to worry about it.

The __format__ part, though, seems kind of daunting.  For one thing, Decimal's implementation looks pretty hairy and scary from my perspective; duplicating it for Fraction would be quite a feat.  For another, the standard format spec doesn't seem to me to cleanly apply to fractional representations--what should be filled, how should alignment work, how would width be divided up, what exactly does precision mean?

I've had a thought, though; what if Fraction.__format__ simply looks for a 'D' at the end of format_spec?  If it's found, just return format(self.as_decimal(), format_spec[:-1].  Otherwise, format using to-be-determined rules specific to fractions, including rules for, say, mixed fractions or denominator of a certain size.

Thoughts?
msg168987 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-08-24 08:53
> I think *both* proposals are sensible. Fraction already has .from_decimal
> (using Decimal), so .to_decimal (also using Decimal) is sensible.

Well, there's a difference: conversion from Decimal to Fraction is well-defined, with a unique, unambiguous result (excluding non-convertibles like infinities and nans);  in particular, the value of any Decimal is exactly representable as a Fraction, so there's little information loss.  (There *is* still some information loss, since Decimal('1.00') and Decimal('1.0') both covert to the same fraction, for example.)

On the other hand, not every Fraction is exactly representable as a Decimal, so the result of conversion from Fraction to Decimal needs information about how many decimal places to produce, what rounding mode to use, what the ideal exponent should be in the case of exact results, etc.  I think Zachary's idea of supporting a context argument, and using the current context if none is supplied, is the way to go here.  The division should end up using an ideal exponent of 0, which doesn't seem unreasonable.

To the patch:  It looks fine, as far as it goes.  It needs tests.  To avoid the repetition of the division code, I'd suggest doing something like:

    if context is None:
        context = getcontext()

Yes, supporting __format__ is going to be a bit more work. :-)
msg169035 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-08-24 16:04
I agree with Mark's arguments. Yesterday I tried to use as_decimal() in
a small program and it did not feel natural to me. I'll probably continue
to use Decimal(f.numerator) / f.denominator.

If this goes in, I'd prefer that as_decimal() always uses a localcontext().
As the patch stands, using as_decimal() pollutes the global context flags, 
which can be quite unexpected:

>>> from fractions import Fraction as F
>>> F(1, 3).as_decimal()
Decimal('0.3333333333333333333333333333')
>>>
>>> import decimal
>>> decimal.getcontext()
Context(prec=28, rounding=ROUND_HALF_EVEN, Emin=-999999, Emax=999999, capitals=1, clamp=0, flags=[Inexact, Rounded], traps=[InvalidOperation, DivisionByZero, Overflow])
>>>


If the function takes a context argument, it might be better to move it
into the decimal module as Decimal.from_fraction(context).


So I'm -1/7 on as_decimal(), but +1 for the __format__() method.
msg169090 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2012-08-24 21:17
(Mark:)
>To the patch:  It looks fine, as far as it goes.  It needs tests.  

I remembered tests about 5 minutes after I submitted the initial patch :P. Here's a patch with some tests.  Note that I've never really done tests, so please let me know if there need to be more/less/different tests.

>To avoid the repetition of the division code, I'd suggest doing something like:
>
>    if context is None:
>        context = getcontext()

(Stefan:)
>If this goes in, I'd prefer that as_decimal() always uses a localcontext().
>As the patch stands, using as_decimal() pollutes the global context flags, 
>which can be quite unexpected

My first attempt was simply:

   def as_decimal(self, context=None):
       with localcontext(context):
           return Decimal(self.numerator) / Decimal(self.denominator)

Looking through decimal.py, it looks like that should work (and in fact it does in 3.2.3), but it seems that _decimal will only accept a context as the argument to localcontext(), not None.  Is that expected, or does that need an issue?

I didn't catch that the patch pollutes the global context flags, that is indeed ugly and unacceptable.

>If the function takes a context argument, it might be better to move it
>into the decimal module as Decimal.from_fraction(context).

Perhaps both?  Implement Decimal.from_fraction(f, context) to do the real work, and then Fraction.to_decimal() as:

   def to_decimal(self):
       return Decimal.from_fraction(self, None)

Those who really care about the context (Decimal users working with a Fraction) can get what they want, and those who don't (Fraction users who want to see a Decimal representation) don't have to bother, and fractions.py isn't polluted with stuff about contexts.
msg169137 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-08-25 14:33
There is indeed a regression in decimal.localcontext with the C accelerator: omitting the argument entirely still works, but passing None explicitly now fails. I created #15783 for that.

On the topic of this thread, I'm also -1 on conversion methods, but +1 on string formatting support.

There's an obvious benefit in providing the latter in order to get a better "feel" for the value of a rational number by displaying a Decimal approximation, but questionable benefit in providing the former.

Explicit conversion is sufficiently rare that I'm OK with the idea of either going via a string (as used to be necessary for binary floats) or by explicitly casting the numerator to Decimal, then dividing by the denominator (no need to cast them both).
msg169148 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-08-25 18:24
After reading the comments, I am ambivalent about a conversion function, and given the lack of consensus or even a clear majority, -1. (Which implies a change in issue title.)

I am +1 on using the new .format() machinery to have fractions format themselves. (Unless Python already does so for other purposes, it should do so without importing decimal.) The uploaded function passes the included test cases. I guess the rounding should be whatever we do for floats. I fudged that in the example.
msg169251 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-08-28 05:51
Thanks Nick.  I'll go ahead and close this one, leaving the Decimal module only with lossless coercions that do not depend on the decimal context (this matches the underlying design of the decimal module which treats all numbers as exact, and only applying to the result of an arithmetic operation).

If needed, a new issue can be opened for a format specifier for the Fraction type.  It seems that we're all in agreement that that would be useful for the fractions module.
History
Date User Action Args
2022-04-11 14:57:31adminsetgithub: 59341
2012-08-28 05:51:08rhettingersetstatus: open -> closed
resolution: rejected
messages: + msg169251
2012-08-25 18:24:20terry.reedysetfiles: + fracdec.py

messages: + msg169148
2012-08-25 14:33:42ncoghlansetnosy: + ncoghlan
messages: + msg169137
2012-08-24 21:17:39zach.waresetfiles: + issue15136_with_tests.patch

messages: + msg169090
2012-08-24 16:04:41skrahsetmessages: + msg169035
2012-08-24 08:53:25mark.dickinsonsetmessages: + msg168987
2012-08-24 08:23:22cvrebertsetnosy: + cvrebert
2012-08-23 18:12:00zach.waresetfiles: + issue15136_as_decimal.patch

nosy: + zach.ware
messages: + msg168954

keywords: + patch
2012-06-23 08:11:07terry.reedysetnosy: + terry.reedy, eric.smith
messages: + msg163546
2012-06-23 07:36:48mark.dickinsonsetmessages: + msg163543
2012-06-23 07:30:16rhettingersetpriority: normal -> low

messages: + msg163540
2012-06-22 16:45:33jonclesetmessages: + msg163433
2012-06-22 16:14:47rhettingersetversions: + Python 3.4, - Python 3.3
2012-06-22 16:13:14rhettingersetassignee: rhettinger
2012-06-22 13:34:28mark.dickinsonsetmessages: + msg163405
2012-06-22 12:49:14mark.dickinsonsetnosy: + rhettinger, skrah
2012-06-22 12:48:32mark.dickinsonsetmessages: + msg163402
versions: + Python 3.3
2012-06-22 12:45:31jonclesetmessages: + msg163401
2012-06-22 12:40:17jonclesetmessages: + msg163400
2012-06-22 12:32:00mark.dickinsonsetnosy: + mark.dickinson
messages: + msg163399
2012-06-22 12:14:38Ramchandra Aptesetnosy: + Ramchandra Apte
messages: + msg163398
2012-06-22 11:51:51jonclecreate