fix atan2() conversions between Sage and SymPy

[certik]

Hi,

please apply the attached patch. The corresponding test is in sympy in sympy/test_external/test_sage.py. It'd be cool to execute that file automatically in sage doctests, not sure currently how to do it.

Component: interfaces

Author: Ondřej Čertík

Reviewer: Burcin Erocal, Karl-Dieter Crisman

Merged: sage-4.5.2.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/8564

[burcin]

Attachment: sympy1.patch.gz

ondrej's patch with a header

[burcin]

Attachment: trac_8564-atan2_conversion.patch.gz

Attachment: trac_8564-doctests.patch.gz

doctest

[burcin]

Reviewer: Burcin Erocal

[burcin]

Author: Ondrej Certik

[burcin]

comment:1

I uploaded two patches,

• attachment: trac_8564-atan2_conversion.patch is Ondrej's fix including a header with a commit message
• attachment: trac_8564-doctests.patch adds a doctest.

I give a positive review to Ondrej's patch, if someone can review the doctest I added this will be ready to go.

The patches to be merged are (in this order):

• attachment: trac_8564-atan2_conversion.patch
• attachment: trac_8564-doctests.patch

The doctest patch depends on #8565.

[kcrisman]

comment:2

This seems fine, works well and tests the appropriate thing (i.e. not arctan2(2,3), but the symbolic thing). Positive review to both.

Question about the Sympy doctest file Ondrej mentions above - it doesn't have

check_expression("atan2(y,x)", "y x")

or whatever would work, in test_functions or something like that. Should it?

[kcrisman]

Changed reviewer from Burcin Erocal to Burcin Erocal, Karl-Dieter Crisman

[certik]

comment:3

Thanks!

Btw, the check_expression() for atan2 is in the latest git sympy, so I need to update the Sage package for it. There were some unrelated mpmath problems with it, that I have to solve first.

Ondrej

[qed777]

Merged: sage-4.5.2.alpha0

[fchapoton]

Changed author from Ondrej Certik to Ondřej Čertík