list_plot should accept lists of complex numbers

[dandrake]

If I do

list_plot([1+I, 2+I, 4-I])

it's totally obvious what I want. We should make list_plot accept lists of complex numbers.

Component: graphics

Keywords: list_plot, complex

Author: Dan Drake

Reviewer: Keshav Kini

Merged: sage-4.8.alpha4

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

[dandrake]

comment:2

Patch up, please review. I improved the docstring for list_plot. I would like feedback on my method for detecting complex numbers. Right now, I do:

from sage.rings.complex_field import ComplexField 
CC = ComplexField() 
from sage.rings.complex_number import is_ComplexNumber 
if any(abs(CC(z).imag()) > 0 or is_ComplexNumber(z) for z in data): 
    data = [(z.real(), z.imag()) for z in data]

But I have a couple concerns:

• do we need to worry about numerical noise? Will testing for abs(imaginary part) > 0 suffice?

• are ComplexField() and is_ComplexNumber the right functions to use here?

Also, we do need to figure this out in list_plot, and not in xydata_from_point_list, because list_plot needs to decide whether to turn the data into tuples of real and imaginary parts, or do zip(range(len(data)), data).

[dandrake]

comment:3

Updated patch. I edited the docstring a bit and improved (I think) the Sphinx markup.

[dandrake]

Author: Dan Drake

[kini]

Reviewer: Keshav Kini

[kini]

comment:5

Nice! All tests pass on sage.math. Patch looks good too, and seems to work fine. Positive review from me.

[dandrake]

comment:6

Keshav: on IRC you were worried about a slowdown, since we do potentially run CC() and is_ComplexNumber on every element of the list. Do you think this will be okay?

[dandrake]

comment:7

Oops: found a bug in my patch. This list doesn't work:

[int(1), CC(2+I),...]

because "'int' object is not callable". Improved patch in a moment.

[kini]

comment:9

Wow, that's ugly. I didn't even think of that... rerunning doctests just in case.

[kini]

comment:10

As for the slowdown, I wasn't really worried, just asking if you thought it was a problem. If you have some doubts about it, by all means, do post to sage-devel :) If so I'll wait before rereviewing this.

[dandrake]

comment:11

Okay, running CC on everything is a terrifically bad idea: here's the current (4.7.2) behavior:

sage: foo = [random() for _ in range(1000)]      
sage: %timeit p = list_plot(foo)                 
625 loops, best of 3: 596 µs per loop
sage: %timeit p = list_plot([CC(z) for z in foo])
125 loops, best of 3: 7.2 ms per loop

So, an order of magnitude slowdown. As suggested by William at https://groups.google.com/d/topic/sage-devel/UiKTG3FkRwY/discussion, the new patch (up shortly) just catches a TypeError, which preserves the current speed for real input and neatly avoids the problem with [int(1), CC(2+I),...]; complex input anywhere will throw the TypeError, which we catch.

[dandrake]

comment:12

Attachment: trac_12035.patch.gz

New patch up. Passes doctests, docs build with no warning. Please review.

[kini]

comment:13

Whoops, sorry. Positive review.

[jdemeyer]

Merged: sage-4.8.alpha4