BUG: floating types should override tp_print #10763
Conversation
ahaldane
force-pushed
the
fix_float_repr_tp_print_override
branch
from
b316305
to
3a10dc6
Compare
numpy/core/tests/test_scalarprint.py
Outdated
| output = f.read() | ||
| f.close() | ||
| assert_equal(output, '0.1999999999999\n') | ||
| # (compare to "print 0.1999999999999" in python2) |
There was a problem hiding this comment.
Can you say what the output is here, rather than making the reader run it?
numpy/core/tests/test_scalarprint.py
Outdated
| @@ -45,6 +46,20 @@ def check(v): | |||
| check(1e15) | |||
| check(1e16) | |||
|
|
|||
| # gh-10753 | |||
There was a problem hiding this comment.
I think this should be its own test function - it doesn't really test the same thing.
| /* | ||
| * In python2, the `float` type still implements the obsolete "tp_print" | ||
| * method, which uses CPython's float-printing routines to print the float. | ||
| * Numpy's floating types PyFloatingArr_Type inherit from float, but override |
There was a problem hiding this comment.
Not true - only PyDoubleArr_Type does - perhaps just replace that one?
ahaldane
force-pushed
the
fix_float_repr_tp_print_override
branch
from
3a10dc6
to
52ca71f
Compare
|
Thanks, updated. |
| * In python3 the tp_print method is reserved/unused. | ||
| */ | ||
| static int | ||
| float_print(PyObject *o, FILE *fp, int flags) |
There was a problem hiding this comment.
Please pick a name that differs from the name used by the builtin float type. Something like doubletype_print would be a reasonable choice and consistent with the other PyDoubleArrType_Type methods.
numpy/core/tests/test_scalarprint.py
Outdated
| f.seek(0) | ||
| output = f.read() | ||
| f.close() | ||
| assert_equal(output, '0.1999999999999\n') |
There was a problem hiding this comment.
Perhaps assert equality with str(x) + '\n'? Not sure if there ever can be rounding issues here, but what we care about is that str and print give the same result...
ahaldane
force-pushed
the
fix_float_repr_tp_print_override
branch
2 times, most recently
from
68dff2d
to
b2f99d0
Compare
numpy/core/tests/test_scalarprint.py
Outdated
| print(x, file=f) | ||
| f.seek(0) | ||
| output = f.read() | ||
| f.close() |
There was a problem hiding this comment.
Can you use a with statement here?
ahaldane
force-pushed
the
fix_float_repr_tp_print_override
branch
from
b2f99d0
to
0d6296a
Compare
|
Thanks Allan. |
| return -1; | ||
| } | ||
|
|
||
| ret = PyObject_Print(to_print, fp, flags); |
There was a problem hiding this comment.
This is wrong, and adds extra quotes.
Should be PyObject_Print(to_print, fp, Py_PRINT_RAW);
Fixes #10753
It turns out that (in python2 only) python's
floattype implements thetp_printmethod slot, and we inherit it in numpy float types. This is a mostly obsolete method which overridesstrandreprin some situations. Its a bit of an anachronism thatfloatimplements it.Since we now override the float str/repr in numpy (with dragon4), we should also override
tp_print. This PR does that.In case it is useful, here is the python2 doc on
tp_print: (link)This might be a candidate for putting in a 1.14.3 release, though maybe this is too small a bug for that.