Implement __format__ for Fraction

[suutari]

BPO	23602
Nosy	@rhettinger, @mdickinson, @scoder, @ericvsmith, @ezio-melotti, @skrah, @vadmium, @serhiy-storchaka, @wm75, @skirpichev, @suutari
Files	issue23602.patch: Fraction.format implementation, test cases and documentation issue23602v2.patch issue23602v3.patch issue23602v4.patch: Fraction.format implementation, test cases and docs, Decimal.format Py and C API unification with test cases

^{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/serhiy-storchaka'
closed_at = None
created_at = <Date 2015-03-07.17:06:09.308>
labels = ['type-feature', 'library']
title = 'Implement __format__ for Fraction'
updated_at = <Date 2021-04-23.08:41:52.382>
user = 'https://github.com/suutari'

bugs.python.org fields:

activity = <Date 2021-04-23.08:41:52.382>
actor = 'Sergey.Kirpichev'
assignee = 'serhiy.storchaka'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2015-03-07.17:06:09.308>
creator = 'tuomas.suutari'
dependencies = []
files = ['38378', '38394', '38413', '38728']
hgrepos = []
issue_num = 23602
keywords = ['patch']
message_count = 29.0
messages = ['237460', '237461', '237468', '237524', '237525', '237526', '237575', '237579', '237583', '237714', '237715', '238813', '239048', '239056', '239336', '239337', '239338', '239342', '239495', '239498', '239501', '239502', '239503', '239504', '239515', '239594', '239664', '239668', '239733']
nosy_count = 11.0
nosy_names = ['rhettinger', 'mark.dickinson', 'scoder', 'eric.smith', 'ezio.melotti', 'skrah', 'martin.panter', 'serhiy.storchaka', 'wolma', 'Sergey.Kirpichev', 'tuomas.suutari']
pr_nums = []
priority = 'low'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue23602'
versions = ['Python 3.5']

Linked PRs

• gh-67790: Support float-style formatting for Fraction instances #100161
• gh-67790: Support basic formatting for Fraction #111320

[suutari]

Since Decimal supports __format__, it would be nice that Fraction did too.

[suutari]

Here's a patch that adds Fraction.__format__ implementation, test cases and documentation.

[serhiy-storchaka]

>>> from fractions import Fraction as F
>>> format(F(1, 3), '.30f')
'0.333333333333333333333333333300'

[suutari]

Serhiy Storchaka wrote:
>>>> from fractions import Fraction as F
>>>> format(F(1, 3), '.30f')
> '0.333333333333333333333333333300'

Good catch! I'll try to fix this and add some more test cases.

[ericvsmith]

I'm not sure it needs fixing: it follows from the definition of using Decimal(num) / Decimal(denom). Plus, it's controllable with a decimal context:

>>> from decimal import localcontext
>>> with localcontext() as ctx:
...   ctx.prec = 100
...   format(F(1, 3), '.30f')
...
'0.333333333333333333333333333333'
>>>

For all of the tests, I suggest using format(value, str) instead of ''.format(value). It more directly tests Fraction.__format__.

In general I think adding Fraction.__format__ is a good idea, and I think converting to Decimal is reasonable for the specified codes. My only question is what to do when "natively" formatting Fractions themselves. We might want to support field widths, padding, etc.

[vadmium]

I’ve never actually used the Fraction class, but I doubt its behaviour should depend on whatever settings are in the current decimal context. Maybe you can extract the precision out of the format string, and base the internal decimal object on that.

[suutari]

Eric V. Smith wrote:

I'm not sure it needs fixing: it follows from the definition of using Decimal(num) / Decimal(denom). Plus, it's controllable with a decimal context:

Hmm... Even though it's tempting to agree with you and just ignore the
precision bug, but to be honest I have to agree with Martin Panter
here. Depending on the current decimal context is not the way of
"Least Surprise" when formatting Fractions.

For all of the tests, I suggest using format(value, str) instead of ''.format(value). It more directly tests Fraction.__format__.

I agree. Will change those.

In general I think adding Fraction.__format__ is a good idea, and I think converting to Decimal is reasonable for the specified codes. My only question is what to do when "natively" formatting Fractions themselves. We might want to support field widths, padding, etc.

Thanks! Actually I already tried to support field widths, padding and
such. (See the test cases.) Or what do you mean?

[suutari]

Here's the next round of the patch.

For formatting fractions with any given precision I had to parse the precision from format specifier and at this point it seemed easier to just create a general parser for the Format Specification Mini-Language. In this patch it is implemented in fractions._parse_format_specifier function, but maybe this kind of general function should be moved to better place and be documented and exported. What do you think?

[vadmium]

Regarding sharing fractions._parse_format_specifier(), perhaps have a look at _pydecimal._parse_format_specifier()

[suutari]

Martin Panter wrote:

Regarding sharing fractions._parse_format_specifier(), perhaps have a look at _pydecimal._parse_format_specifier()

I did find that, but since it was a private function in private
module, I was unsure if I can use it here. The _pydecimal one parser
also does more stuff that I need.

[suutari]

Version 3 of the patch. Changes to v2:

• Use raw-strings for the regexps.
• Make the specifier regexp more robust with \A, \Z and re.DOTALL.
• Add more test cases; especially g and e formats and negative fractions.

[serhiy-storchaka]

>>> from fractions import Fraction as F
>>> format(F(4, 27), 'f')
'0.1481481'
>>> format(F(4, 27), '.1f')
'0.2'

[mdickinson]

[Eric]

I'm not sure it needs fixing...

Hmm. We've gone to some lengths to make sure that we get correctly-rounded results for formatting of Decimal and float types, as well as to make sure that operations like converting a Fraction to a float are correctly rounded. It would be disappointing if the result of formatting a Fraction wasn't correctly rounded, and I'd personally consider it a bug.

[scoder]

Absolutely. Fractions are all about exact calculations, much more so than Decimals. So the formatting output should be as accurate as requested or possible (well, excluding infinity).

[wm75]

actually, I'm not sure whether formatting Decimals gives correct output under all conditions (correctly rounded yes, but maybe not formatted correctly?).

compare:

>>> format(float('1.481e-6'),'.3g')
'1.48e-06'

>>> format(Decimal('1.481e-6'),'.3g')
'0.00000148'

>>> format(float('1.481e-7'),'.3g')
'1.48e-07'

>>> format(Decimal('1.481e-7'),'.3g')
'1.48e-7'

So with the 'g' specifier the switch between floating point and scientific notation seems to be broken.
Also note the slightly different formatting of the exponent: the leading zero is missing for all specifiers, i.e. 'g', 'G', 'e', 'E'.

Are these bugs ?

[skrah]

Decimal formatting intentionally differs from float formatting, see bpo-23460.

[wm75]

Decimal formatting intentionally differs from float formatting, see bpo-23460.

I see. Thanks for the pointer. What about the missing zero in the exponent ?

[skrah]

The zero isn't missing. :) We are following http://speleotrove.com/decimal/decarith.html, with thousands of test cases.

We could decide to do something special for "g", but there are good reasons not to do that.

[suutari]

Thanks for the comments again!

I fixed the "format(F(4, 27), '.1f') -> 0.2" issue
Serhiy Storchaka reported. Fix for that was as simple as adding one to the precision the decimals are calculated in, but while adding test cases for that I realized two new things:

(a) I don't want "f" specifier to mean "infinite" precision, but instead some predefined value. I chose 6.
(b) How about rounding? I don't want the current decimal context to affect that, since it's not logical that formatting of Fractions depends on the decimal context.

The rounding thing made things harder, since there was no way to pass decimal context for Decimal.__format__ without changing the local context -- at least with the C implementation; the Python implementation (pydecimal) provided nicer API with optional context keyword argument. So I decided to unify the C and Py API's of Decimal.__format_ and add the keyword argument support to the C API too. This is done in this v4 of the patch.

There's no docs for the added Decimal.__format__ kwargs, since I want some comments on that change first.

[serhiy-storchaka]

I think that Decimal is not needed for Fraction.__format__ (and I'm not sure that issue23602v4.patch is correct). The correct way to format Fraction as fixed-precision decimal is to use Fraction.__round__() or similar algorithm. The implementation can look like:

    f = self.__round__(prec)
    i = int(f)
    return '%d.%0*d' % (i, prec, abs(f - i) * 10**prec)

[suutari]

On 29 March 2015 at 19:54, Serhiy Storchaka wrote:

I think that Decimal is not needed for Fraction.__format__ (and I'm not sure that issue23602v4.patch is correct).

Of course it's not needed. I'm using it to avoid re-implementing all
the various formatting variations that can be controlled with the
fill/align/sign/width/,/precision/type parameters
(https://docs.python.org/3/library/string.html#formatspec). IMHO those
should be supported as they are with floats and Decimals.

The correct way to format Fraction as fixed-precision decimal is to use Fraction.__round__() or similar algorithm. The implementation can look like:

f = self.\_\_round__(prec)
i = int(f)
return '%d.%0\*d' % (i, prec, abs(f - i) * 10\*\*prec)

Why this would be more correct than delegating the rounding (and
formatting) to Decimal.__format__? (Then we just have to make sure
that we have enough precision in the decimal context we're operating
in. That's what I got wrong in the previous round.)

[scoder]

But these parameters could also be partly delegated to normal string (not number) formatting, right?

One of the advantages of not depending on Decimal is, well, to not depend on Decimal, which is a rather uncommon dependency when using Fractions in an application.

I think it could avoid some more calculations to first multiply the nominator by 10**prec, then round(), int() and str() the result, and then split the string at "-prec".

BTW, if "division with remainder" wasn't (sadly) linear time, that would definitely be the most beautiful algorithm here. :)

[scoder]

Meaning, something like this should work:

    x = (nom * 10**(prec+1)) // den
    if x % 10 < 5:
       x = x // 10
    else:
       x = x // 10 + 1
    print('%s.%s' % (x[:-prec], x[-prec:]))

[scoder]

Or, speaking of "division with remainder":

n, r = divmod(nom * 10**prec, den)
if r * 5 >= den:
    n += 1
x = str(n)
print('%s.%s' % (x[:-prec], x[-prec:]))

... minus the usual off-by-one that the tests would quickly find :)

[wm75]

Initially, I also thought that this should be addressable with Fraction.__round__ or an optimized variation of it, but Tuomas is right that it gets complicated by the fact that you need to cope with the different format specifiers and not all of them fit the algorithm.
In particular, scientific notation poses a problem because it may require a lot more precision in the calculation than the one finally used for formatting. Consider:

>>> float(round(Fraction(4, 27000), 6))
0.000148

but
>>> format(4/27000, 'e')
'1.481481e-04'

Trying to deal with this in pure Python quickly slows your code (at least a few naive attempts of mine) unacceptably compared to Tuomas' patch.

[skrah]

Regarding Decimal:

1. The context precision isn't used for formatting. If you have
   another reason for proposing the optional context argument for
   dec_format(), please open another issue.

2. There's another problem: The mythical DefaultContext (which
   acts as a template for creating new contexts) affects not only
   new thread-local contexts, but also a new Context()!

   In my opinion this is something we should change: The mechanism
   is fine for thread-local contexts, but Context() should behave
   like a pure function.

3. The double rounding issues are more tricky than it might seem;
   if we use Decimal for this, perhaps direct support in the module
   would be the cleanest option.

[suutari]

On 30 March 2015 at 13:49, Stefan Krah wrote:

Regarding Decimal:

1. The context precision isn't used for formatting. If you have
   another reason for proposing the optional context argument for
   dec_format(), please open another issue.

Yes, context precision isn't, but context rounding mode is. That's why
I wanted to use a known context rather than the current (thread-local)
context.

But yes, I also thought that maybe the Decimal.__format__ changes
should go to another issue, which my implementation would then depend
on though. It wouldn't be too bad if Py and C version of
Decimal.__format__ had same interface. What do you think?

2. There's another problem: The mythical DefaultContext (which
   acts as a template for creating new contexts) affects not only
   new thread-local contexts, but also a new Context()!

   In my opinion this is something we should change: The mechanism
   is fine for thread-local contexts, but Context() should behave
   like a pure function.

I don't understand what do you mean with this. Is this something that
I'm doing wrong in my patch or just another (related?) issue?

3. The double rounding issues are more tricky than it might seem;
   if we use Decimal for this, perhaps direct support in the module
   would be the cleanest option.

What double rounding issues you're referring to?

[vadmium]

I understand double rounding to mean incorrectly rounding something like 0.149999 up to 0.2. It should be rounded once to 1 decimal place (0.1). If you temporarily round it to a higher number of places before rounding to 1 place, you’re doing it wrong. So you might have to ensure that any rounding done before formatting step exactly matches the rounding specified in the formatting.

[skrah]

It wouldn't be too bad if Py and C version of Decimal.__format__ had
same interface. What do you think?

Let's discuss that in a separate issue.

[DefaultContext]

I don't understand what do you mean with this. Is this something that
I'm doing wrong in my patch or just another (related?) issue?

Decimal.DefaultContext has global scope and currently affects
Context() creation, unless you specify all parameters.

[mdickinson]

Done for float-style formatting in #100161. I'm planning to make a new PR that adds support for the d presentation format before closing this issue.

[mdickinson]

I'm planning to make a new PR that adds support for the d presentation format before closing this issue.

See #111320.

[skirpichev]

@mdickinson, #111320 was merged. From the issue thread it seems one could be closed.

[mdickinson]

@skirpichev It could indeed! Thanks.