Issue1600162

classification
Title: cmath implementation
Type: Severity: normal
Components: None Versions:
Milestone:
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: Nosy List: cgroves, tfreund
Priority: normal Keywords: patch

Created on 2006-11-21.04:02:45 by tfreund, last changed 2006-12-05.02:58:59 by cgroves.

Files
File name Uploaded Description Edit Remove
cmath.patch tfreund, 2006-12-02.23:50:42 cmath module incorporating cgrove's suggestions
test_cmath.py tfreund, 2006-12-03.23:17:03
Messages
msg2548 (view) Author: Tim Freund (tfreund) Date: 2006-11-21.04:02:45
Here is an implementation of the cmath module.  The logic was lifted directly from the CPython implementation, and the code was formatted with Eclipse's Java Conventions formatter.

The tests included in CPython are pretty weak - they just make sure that the functions return.  Adding more comprehensive tests would be a good thing.

This is my first module implementation, so I am sure there is room for improvement. Comments are appreciated.

Best regards,

Tim

msg2549 (view) Author: Charlie Groves (cgroves) Date: 2006-11-22.03:15:57
This looks like a good start,  but I do have a few comments.

We're not using Java 5 yet, so your complexFromObject can't use
autounboxing to get Double and Integer objects into PyComplex.  That
doesn't actually matter though because there's an easier
implementation of complexFromObject.  Java methods exposed to Jython
can just take PyObject and Jython will do the right thing in looking
them up and calling them.  PyObject has a __complex__ method on it
just like the special method in Python that returns a complex version
of a given object.  The only thing you have to do other than calling
__complex__ is check for an AttributeError being raised.  That's what
PyObject does by default, so if a subclass hasn't implemented
__complex__, you need to handle that and throw a type error instead.
It'd look something like:

       try {
           return in.__complex__();
       } catch(PyException pyEx) {
           if(pyEx.type == Py.AttributeError) {
               throw Py.TypeError("a float is required");
           }
           throw pyEx;
       }

So you can just replace all of your Object in arguments with PyObject
in and you should be good to go.

I'm not actually looking at your implementation of complex math.  I'm
assuming you can transcribe it from C pretty well and I don't actually
want to remember how to work with complex numbers.  It would be nice
if you'd expand on test_cmath.py to actually exercise this.  I'm sure
the CPython people wouldn't mind an expanded test suite.
msg2550 (view) Author: Tim Freund (tfreund) Date: 2006-12-02.23:50:42
I found some time to look at both Jakarta Commons Math and Ruby's complex module for some validation of our cmath implementation.  I am now fairly confident that it behaves correctly.  I will submit the changed test_cmath.py to the Python project.

I have attached a new cmath.patch file that incorporates the changes suggested by Charlie Groves.

Thanks,

Tim
msg2551 (view) Author: Charlie Groves (cgroves) Date: 2006-12-03.22:58:10
Looks good.  Could you attach your test_cmath.py as well?  I'd like to add it to our svn at the same time as the rest of the patch.
msg2552 (view) Author: Tim Freund (tfreund) Date: 2006-12-03.23:17:03
Here's the new test_cmath.py.
msg2553 (view) Author: Charlie Groves (cgroves) Date: 2006-12-05.02:58:59
Looks good!  Committed in r3006.
History
Date User Action Args
2006-11-21 04:02:45tfreundcreate