classification
Title: float.fromhex discrepancy under Solaris
Type: behavior Stage:
Components: Interpreter Core Versions: Python 3.0, Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: benjamin.peterson, mark.dickinson, pitrou
Priority: critical Keywords: needs review, patch

Created on 2008-08-21 13:18 by pitrou, last changed 2009-04-18 17:56 by mark.dickinson. This issue is now closed.

Files
File name Uploaded Description Edit
issue3633_test.patch mark.dickinson, 2008-08-21 19:52 Patch to make tests produce more info on failure
issue3633.patch mark.dickinson, 2008-08-21 21:27 Possible fix
Messages (14)
msg71635 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-08-21 13:18
This is a failure that seems to occur quite often (always?) on the
Solaris buildbot:

======================================================================
FAIL: test_invalid_inputs (test.test_float.HexFloatTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File
"/home2/buildbot/slave/3.0.loewis-sun/build/Lib/test/test_float.py",
line 451, in test_invalid_inputs
    self.assertRaises(ValueError, fromHex, x)
AssertionError: ValueError not raised by fromhex

----------------------------------------------------------------------
msg71640 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-08-21 13:39
This probably has been happening for quite a while. We just didn't
notice because that bot was busy hanging on test_nis.

The failure is in test_math. Mark, do you know anything?
msg71644 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-08-21 14:51
I'll take a look, though if anyone has some time to spare and access to a 
Solaris machine then they can probably figure this out more quickly.

The first step would be to fix the test so that it at least shows which 
input the failure occurs for.
msg71672 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-08-21 19:52
Here's a patch for the test-suite to get more information about where 
float.fromhex is failing.  Could someone else please review this quickly 
so that I can check it in?  It shouldn't take more than a few minutes to 
review.
msg71673 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-08-21 19:55
That looks fine.
msg71675 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-08-21 20:07
Thanks, Benjamin!  Change to test committed in r65958, merged to py3k in 
r65959.  Time to watch the py3k solaris buildbot.
msg71681 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-08-21 21:12
The problem appears to be that on Solaris, the isxdigit function (which 
is supposed to check whether a character is a valid hex digit) returns 
true for Unicode fullwidth digits.  On other systems I have access to, 
isxdigit just returns true for the ASCII hex digits, and you use the C99 
iswxdigit function if you want to allow other Unicode digits.

One could argue that these fullwidth digits should be permitted, but I 
don't know any quick way to convert a unicode digit to its value.  For 
now, it just seems simplest to replace the isxdigit call with an 
explicit check for the ASCII 7-bit characters '0' through '9', 'a' 
through 'f'.
msg71683 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-08-21 21:14
Le jeudi 21 août 2008 à 21:12 +0000, Mark Dickinson a écrit :
> For now, it just seems simplest to replace the isxdigit call with an 
> explicit check for the ASCII 7-bit characters '0' through '9', 'a' 
> through 'f'.

+1
I don't think it makes a lot of sense to support full width unicode
digits. Hex representation is quite specialized, it won't be produced by
casual users.
msg71685 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-08-21 21:26
Here's a patch.  I'm reasonably confident that this should fix the 
problem, but I don't have a Solaris machine to test it on.

If anyone can check this on Solaris that would be fantastic, but I'll 
settle for a 'looks okay to me' from another core developer.
msg71686 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-08-21 21:27
...and here's the actual patch!
msg71688 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-08-21 21:32
Le jeudi 21 août 2008 à 21:26 +0000, Mark Dickinson a écrit :
> Here's a patch.  I'm reasonably confident that this should fix the 
> problem, but I don't have a Solaris machine to test it on.
> 
> If anyone can check this on Solaris that would be fantastic, but I'll 
> settle for a 'looks okay to me' from another core developer.

Looks okay to me.
msg71689 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-08-21 21:45
Thanks, Antoine.  Committed, r65964 (2.6) and r65965 (3.0).  Setting 
status to pending;  will close if/when the Solaris buildbot goes green.
msg71717 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-08-22 01:59
The buildbot is still failing, but not on test_math.
msg86128 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-18 17:56
A postscript:  looking back at this from afar, the original error was 
almost certainly due to a missing Py_CHARMASK around the arguments
to isdigit and isxdigit, and nothing to do with Unicode fullwidth digits 
at all.
History
Date User Action Args
2009-04-18 17:56:16mark.dickinsonsetmessages: + msg86128
2008-08-22 01:59:38benjamin.petersonsetstatus: pending -> closed
messages: + msg71717
2008-08-21 21:45:13mark.dickinsonsetstatus: open -> pending
resolution: fixed
messages: + msg71689
2008-08-21 21:32:42pitrousetmessages: + msg71688
2008-08-21 21:31:45mark.dickinsonsetkeywords: + needs review
2008-08-21 21:27:17mark.dickinsonsetfiles: + issue3633.patch
messages: + msg71686
2008-08-21 21:26:42mark.dickinsonsetmessages: + msg71685
2008-08-21 21:14:45pitrousetmessages: + msg71683
2008-08-21 21:12:27mark.dickinsonsetmessages: + msg71681
2008-08-21 20:07:59mark.dickinsonsetkeywords: - needs review
messages: + msg71675
2008-08-21 19:55:50benjamin.petersonsetmessages: + msg71673
2008-08-21 19:52:41mark.dickinsonsetkeywords: + needs review, patch
files: + issue3633_test.patch
messages: + msg71672
2008-08-21 14:51:27mark.dickinsonsetassignee: mark.dickinson
messages: + msg71644
2008-08-21 13:39:57benjamin.petersonsetnosy: + mark.dickinson, benjamin.peterson
messages: + msg71640
2008-08-21 13:18:34pitroucreate