classification
Title: "%d" format handling for long values
Type: Stage:
Components: Interpreter Core Versions: Python 2.5
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: facundobatista Nosy List: amaury.forgeotdarc, facundobatista, ggenellina, goodger, phr, pixelbeat, teoliphant
Priority: normal Keywords: patch

Created on 2007-06-25 06:24 by ggenellina, last changed 2008-06-11 21:23 by pixelbeat. This issue is now closed.

Files
File name Uploaded Description Edit
string_format_with_long_values.diff ggenellina, 2007-06-25 06:24 "%d" format handling for long values
floatfmt.diff ggenellina, 2008-02-18 11:43 updated patch
Messages (11)
msg52799 - (view) Author: Gabriel Genellina (ggenellina) Date: 2007-06-25 06:24
String formatting using %d doesn't handle well a long value when the object isn't a PyLong itself. 
That is:

py> "%d" % 42L
'42'
py> "%d" % 10000000000000000000000000000000L
'10000000000000000000000000000000'
py> "%d" % 42.0
'42'
py> "%d" % 10000000000000000000000000000000.0
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: int argument required

Here it was shown with a float object, but any other object with a __long__ or __int__ method returning a PyLong fails (by example, the gmpy package).

Currently PyString_Format, for "%d", checks whether the value *is* a PyLong and processes it, else tries to *convert* it to PyInt (and fails for large values).

The attached patch first checks for a number; if it's a number but not PyInt nor PyLong, tries to convert to int; if it fails, tries to convert to long. The resulting value (either a PyInt or PyLong) is formatted the same way as before. 
If the original object was actually an int or long, it's handled the same way as before.
The test ordering was chosen to only convert to long when necesary.
msg57042 - (view) Author: Travis Oliphant (teoliphant) * (Python committer) Date: 2007-11-01 22:29
I have two issues with this patch:

1) I'm not sure it's that bad to need to use '%d' % long(obj) to ensure
conversion to a long integer.

2) If this kind of auto-conversion is deemed useful, then the patch
itself is rather complicated.   I would re-factor so that the same code
is not repeated once in the PyNumber_Check and again in the original
PyLong_Check and else clauses.  Simply check for PyNumber.  Then, if not
already an int or a long, convert to int first and then long if that
creates an error.  Then, excecute the two segments of code for int and
long objects.
msg57186 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2007-11-06 23:01
I'm positive that this shouldn't happen. There should NOT be any
difference between longs and ints in nowadays Python, so you never
should say to an user to call that long() before the %d.

And, you have some strange behaviours... for example:

>>> "%d" % 9e8
'900000000'
>>> "%d" % 9e9
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: int argument required

Why the first is ok and in the second you should have called it through
long()?

Gabriel, could you please take a look to the recommendations that Travis
is doing? Maybe the patch could be simpler... In any case, please
confirm if yes or no, :)
msg57226 - (view) Author: Gabriel Genellina (ggenellina) Date: 2007-11-07 22:44
Yes, I can reformulate it. In fact my original version was quite 
different, but the resulting diff was hard to understand so I rewrote 
it trying to keep as much as the original code as possible.
I'll submit a new patch next weekend.
msg61636 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-01-24 16:22
Closed issue 1924 as duplicate of this one, but I'm copying here the
text from David, as it's very explanative:

"""
I ran across this bug in some legacy production code when numbers got high:

>>> '%i' % 2e9
'2000000000'
>>> '%i' % 3e9
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
TypeError: int argument required

It looks like the float is being automatically converted to an int, but
floats > sys.maxint cause an error.  However,

>>> int(3e9)
3000000000L

So the implicit float-to-int conversion is imperfect; large floats are
not being converted to long ints.

Same error in Python 2.3 through 2.6a0 (as of 2007-12-28).

In Python 2.1.3 & 2.2.3 the error is "OverflowError: float too large to
convert".  The same error is triggered by int(3e9) though.

While it's arguably not-quite-sane to have code that triggers this
error, the inconsistency is what concerns me.
"""
msg62522 - (view) Author: Gabriel Genellina (ggenellina) Date: 2008-02-18 11:43
An updated patch, along the lines given by Travis Oliphant.
msg62675 - (view) Author: paul rubin (phr) Date: 2008-02-22 04:03
I would prefer that %d signal an error 100% of the time if you give it a
float.  It should not accept 42.0.  It is for printing integers.
msg62681 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-02-22 09:03
I am of the opposite side:
%d should accept floats and anything that can be converted to an integer.
It is for printing objects with a decimal format.
(likewise %s is for printing objects that can be converted to a string)
msg62684 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-02-22 11:54
Paul, %d will accept large floats, I need to review Gabriel's patch;
your proposition will break too much code.
msg62872 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-02-24 03:18
Applied in r61040. Thanks you all!
msg68021 - (view) Author: Pádraig Brady (pixelbeat) Date: 2008-06-11 21:23
A couple of comments.

1. This bug supersedes issue 1153226
That has good info, including the suggestion that one should
be using the %.f format rather than %d in this case anyway

2. The patch here was actually applied in r61041
History
Date User Action Args
2008-06-11 21:23:40pixelbeatsetnosy: + pixelbeat
messages: + msg68021
2008-02-24 03:18:26facundobatistasetstatus: open -> closed
resolution: accepted
messages: + msg62872
2008-02-22 11:54:30facundobatistasetmessages: + msg62684
2008-02-22 09:03:26amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg62681
2008-02-22 04:03:36phrsetnosy: + phr
messages: + msg62675
2008-02-18 11:44:01ggenellinasetfiles: + floatfmt.diff
messages: + msg62522
2008-01-24 16:22:41facundobatistasetnosy: + goodger
messages: + msg61636
2008-01-21 16:39:55mark.dickinsonlinkissue1153226 superseder
2007-11-07 22:44:14ggenellinasetmessages: + msg57226
2007-11-06 23:01:39facundobatistasetmessages: + msg57186
2007-11-01 22:29:05teoliphantsetnosy: + teoliphant
messages: + msg57042
2007-06-25 06:24:36ggenellinacreate