classification
Title: test_float fails for 'legacy' float repr style
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.1
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: eric.smith, mark.dickinson
Priority: normal Keywords: patch

Created on 2009-04-17 10:32 by mark.dickinson, last changed 2009-04-17 22:42 by mark.dickinson. This issue is now closed.

Files
File name Uploaded Description Edit
issue5780.patch mark.dickinson, 2009-04-17 11:15
Messages (8)
msg86066 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-17 10:32
After:

 CC="gcc -DPY_NO_SHORT_FLOAT_REPR" ./configure && make

test_format_testfile in test_float fails with:

test.support.TestFailed: Traceback (most recent call last):
  File "Lib/test/test_float.py", line 341, in test_format_testfile
    self.assertEqual(fmt % float(arg), rhs)
AssertionError: '100000000000.0' != '1e+11'

The problem is that for str(float), the new formatting code switches to 
exponential notation at 1e11, not 1e12; the legacy code does not.

A similar issue exists for repr: repr(1e16) should now produce '1e+16'.
msg86067 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-17 10:34
Here's a patch.  Eric, do you have time to check this over for general 
sanity?
msg86068 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-17 11:15
On second thoughts, I think that repr shouldn't change:  'legacy'
repr format should mean that it *is* exactly the same as before,
including the switch at 1e17 instead of 1e16.  Otherwise there will be 3 
possibilities for repr output: 'short' repr, 'legacy' repr and pre-3.1 
repr will all be different.

Here's a new patch, which leaves repr alone and fixes up the Misc/NEWS 
entries accordingly.
msg86070 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2009-04-17 11:43
I agree that there should be 2 behaviors, not 3.

This patch looks okay to me, and tests pass both with and without
defining PY_NO_SHORT_FLOAT_REPR.

One nit: You don't need to use Py_CHARMASK when using the ISDIGIT macro
locally defined in pystrtod.c. On the other hand, that macro (ISDIGIT)
should completely go away. I think it's there because this file is
trying to be locale-unaware, but isdigit from ctype.h is defined as
being locale-unaware (at least on all platforms I have access to and a
search on the net; my copy of C89 is at home and I can't triple-check
just yet).
msg86071 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2009-04-17 12:09
Also, remove_trailing_zeros should go inside the #ifdef so it's not
included if Gay's code is being used.
msg86072 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-17 12:56
I seem to recall that despite the C standards, there are platforms around
where isdigit and isxdigit are still locale aware.  This bit me when I 
implemented float.fromhex:  see r65964.

And my man page for isdigit says:

COMPATIBILITY
     The 4.4BSD extension of accepting arguments outside of the range of 
the unsigned char type in locales with large character sets
     is considered obsolete and may not be supported in future releases.  
The iswdigit() function should be used instead.


So I'm tempted to go the other way and say that all uses of isdigit should 
be replaced by ISDIGIT.


I'll fix the Py_CHARMASK and move remove_trailing_zeros inside the ifdef.
msg86074 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2009-04-17 13:18
I definitely think that if you go with isdigit, you need the macro. I've
been bitten by this in 3.0.

My actual suggestion is to go with isdigit and the macro everywhere in
this file and all files.

But for just this checkin, ISDIGIT is probably best.
msg86096 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-17 22:42
Committed, with the fixups Eric pointed out, in r71692.
History
Date User Action Args
2009-04-17 22:42:45mark.dickinsonsetstatus: open -> closed
resolution: fixed
messages: + msg86096

stage: patch review -> resolved
2009-04-17 13:18:01eric.smithsetmessages: + msg86074
2009-04-17 12:56:48mark.dickinsonsetmessages: + msg86072
2009-04-17 12:09:49eric.smithsetmessages: + msg86071
2009-04-17 11:43:30eric.smithsetmessages: + msg86070
2009-04-17 11:15:41mark.dickinsonsetfiles: - issue5780.patch
2009-04-17 11:15:33mark.dickinsonsetfiles: + issue5780.patch

messages: + msg86068
2009-04-17 10:34:08mark.dickinsonsetfiles: + issue5780.patch
keywords: + patch
messages: + msg86067
2009-04-17 10:32:52mark.dickinsoncreate