Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

real and imag of complex should return float #15032

Closed
eviatarbach opened this issue Aug 10, 2013 · 9 comments
Closed

real and imag of complex should return float #15032

eviatarbach opened this issue Aug 10, 2013 · 9 comments

Comments

@eviatarbach
Copy link

real(z) and imag(z), where z is complex, as of now returns a complex. It should return a float, since that makes it consistent with real(CC(3, 4)), for example. It also fixes problems with plotting the real or imaginary parts of complex functions.

Component: symbolics

Author: Eviatar Bach

Reviewer: Punarbasu Purkayastha

Merged: sage-5.12.beta3

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

@eviatarbach eviatarbach added this to the sage-5.12 milestone Aug 10, 2013
@eviatarbach
Copy link
Author

comment:1

Patchbot apply trac15032.2.patch

@ppurka
Copy link
Member

ppurka commented Aug 11, 2013

comment:2

The new functions needs some doctests to check that the type is correct.

I am actually quite surprised by the inconsistency of the Sage implementation. self.real contains the number in the case of python's complex but it is a function in Sage's implementation. Do you know what is the reason behind the function in the Sage implementation?

sage: a = CC(1)     
sage: a.real
<function real>
sage: a = complex(1)
sage: a.real
1.0

EDIT Actually just shift the doctests to within the __call__ functions.

@eviatarbach
Copy link
Author

comment:3

Okay, I'll add the tests.

I think the reason for the functions is that it's converting from MPFR. It could be changed with the @property decorator so that self.real will work.

@eviatarbach
Copy link
Author

Attachment: trac15032.3.patch.gz

@eviatarbach
Copy link
Author

comment:4

I added the tests to __call__ but also left the other ones intact, since the __call__ ones aren't visible in the documentation.

@ppurka
Copy link
Member

ppurka commented Aug 12, 2013

comment:5

Looks good to me. All tests pass in sage/rings and sage/functions

Patchbot apply trac15032.3.patch

@ppurka
Copy link
Member

ppurka commented Aug 12, 2013

Author: Eviatar Bach

@ppurka
Copy link
Member

ppurka commented Aug 12, 2013

Reviewer: Punarbasu Purkayastha

@jdemeyer
Copy link

Merged: sage-5.12.beta3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants