[Jython-bugs] [ jython-Patches-1600162 ] cmath implementation

[SourceForge.net <no...@so...>]

Patches item #1600162, was opened at 2006-11-20 23:02
Message generated for change (Comment added) made by cgroves
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=312867&aid=1600162&group_id=12867

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
>Status: Closed
>Resolution: Accepted
Priority: 5
Private: No
Submitted By: Tim Freund (tfreund)
Assigned to: Nobody/Anonymous (nobody)
Summary: cmath implementation

Initial Comment:
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



----------------------------------------------------------------------

>Comment By: Charles Groves (cgroves)
Date: 2006-12-04 21:58

Message:
Logged In: YES 
user_id=1174327
Originator: NO

Looks good!  Committed in r3006.

----------------------------------------------------------------------

Comment By: Tim Freund (tfreund)
Date: 2006-12-03 18:17

Message:
Logged In: YES 
user_id=150918
Originator: YES

Here's the new test_cmath.py.

----------------------------------------------------------------------

Comment By: Charles Groves (cgroves)
Date: 2006-12-03 17:58

Message:
Logged In: YES 
user_id=1174327
Originator: NO

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.

----------------------------------------------------------------------

Comment By: Tim Freund (tfreund)
Date: 2006-12-02 18:50

Message:
Logged In: YES 
user_id=150918
Originator: YES

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

----------------------------------------------------------------------

Comment By: Charles Groves (cgroves)
Date: 2006-11-21 22:15

Message:
Logged In: YES 
user_id=1174327
Originator: NO

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.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=312867&aid=1600162&group_id=12867