[Python-Dev] PEP 485 isclose() implementation review requested (original) (raw)
Chris Barker chris.barker at noaa.gov
Mon May 18 01:02:49 CEST 2015
- Previous message (by thread): [Python-Dev] PEP 484 wishes
- Next message (by thread): [Python-Dev] PEP 485 isclose() implementation review requested
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Folks,
After a huge delay, I finally found the time to implement the PEP 485 isclose() function, in C. I tihnk it's time for some review. I appologise for the fact that I have little experience with C, and haven't used the raw C API for years, but it's a pretty simple function, and there's lots of code to copy, so I think it's in OK shape. I hav not yet integrated it with the cPyton source code -- it belongs in mathmodule.c, but I found it easier to put it in a separate module while figuring it out.
You can find the code in the same gitHub repo as the draft PEP and python prototyping code:
https://github.com/PythonCHB/close_pep
the code is in: is_close_module.c
There is a test module in: test_isclose_c.py
and it can be built with:
python3 setup.py build_ext --inplace
Let me know if I should restructure it or put it anywhere else before it gets reviewed but in the meantime, i welcome any feedback.
Thanks, -Chris
A few questions I have off the bat:
C-API (and plain C) questions:
Is there a better way to create a False or True than::
PyBool_FromLong(0) and PyBool_FromLong(1)
Style question: should I put brackets in an if clause that has only one line?::
if (some_check) { just_this_one_expression }
I can't find docs for PyDoc_STRVAR: but it looks like it should use it -- how?
I'm getting a warning in my PyMethodDef clause::
static PyMethodDef IsCloseMethods[] = { {"isclose", isclose_c, METH_VARARGS | METH_KEYWORDS, "determine if two floating point numbers are close"}, {NULL, NULL, 0, NULL} /* Sentinel */ };
is_close_module.c:61:17: warning: incompatible pointer types initializing 'PyCFunction' (aka 'PyObject ()(PyObject *, PyObject *)') with an expression of type 'PyObject *(PyObject *, PyObject *, PyObject *)' [-Wincompatible-pointer-types] {"isclose", isclose_c, METH_VARARGS | METH_KEYWORDS,
but it seems to be working -- and I've copied, as well as I can, the examples.
Functionality Questions:
What do do with other numeric types?
- Integers cast fine...
- Decimal and Fraction cast fine, too -- but precision is presumably lost.
- Complex ? -- add it to cmath?
It's raising an Exception for negative tolerances: which don't make sense, but don't really cause harm either (fabs() is used anyway). I can't say I recall why I did that for the python prototype, but I reproduced in the C version. Should I?
What about zero tolerance? should equal values still be considered close? They are now, and tests reflect that.
--
Christopher Barker, Ph.D. Oceanographer
Emergency Response Division NOAA/NOS/OR&R (206) 526-6959 voice 7600 Sand Point Way NE (206) 526-6329 fax Seattle, WA 98115 (206) 526-6317 main reception
Chris.Barker at noaa.gov -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.python.org/pipermail/python-dev/attachments/20150517/bdd265e1/attachment.html>
- Previous message (by thread): [Python-Dev] PEP 484 wishes
- Next message (by thread): [Python-Dev] PEP 485 isclose() implementation review requested
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]