[with patch, positive review] xmin/xmax now broken in plot()

[bober]

r(t) = 1000 * t * e^(-.5 * t)
plot(r, xmin=0, xmax=20).show()

doesn't work. But

plot(r, (0,20)).show()

does. The documentation still says

    PLOT OPTIONS:
    The plot options are
    [...]
        xmin -- starting x value
        xmax -- ending x value
    [...]

[mabshoff]

This is still broken in Sage 3.0.alpha1.

Cheers,

Michael

[mhampton]

In addition to fixing this problem, I also enforced the xmin, xmax that are given if they are inside [-1,1].

[mhampton]

should fix the problem

[jhpalmieri]

Looks okay to me, although I don't understand the purpose of the change from

G = Graphics() 
for i in range(0, len(funcs)): 

to

G = plot(funcs[0], (xmin, xmax), polar=polar, **kwds) 
for i in range(1, len(funcs)): 

[mhampton]

The purpose of that change was: I was trying to avoid the xmin of the output being set to -1 if the argument xmin was greater than that. If you initialize something as Graphics(), xmin is set to -1.

There is probably a better systematic way of solving that problem but I don't have it in hand.

[jhpalmieri]

Hm. If I do:

sage: r(t) = 1000 * t * e^(-0.5*t)
sage: plot(r, xmin=10, xmax=20).show()

or (to make sure I didn't misunderstand "greater than" -1):

sage: r(t) = 1000 * t * e^(-0.5*t)
sage: plot(r, xmin=-2, xmax=20).show()

then I seem to get the same behavior with or without this particular change in the code.

I have two more questions: what should the following do?

plot (r, xmin=10, xmax=-2).show()

It should probably print an error (since xmin > xmax), but right now I get a graph which is a bad approximation to

plot (r, xmin=-2, xmax=10).show()

It's actually pretty strange looking...

Also, do you have any idea why, if I do

plot (r, xmin=5, xmax=20).show()

then the vertical axis is the line x=5, not x=0? When xmin is positive, I seem to get x=xmin as the vertical axis, which looks strange to me. I guess the same happens if both xmin and xmax are negative: then x=xmax is drawn as the vertical axis. (This is probably a completely separate issue, but I thought I'd ask.)

[gfurnish]

This seems correct except for an error check, can we get a patch?

[jhpalmieri]

Here's a new version of the patch. This (I hope) takes the arguments called xmin and xmax, and sets xmin to be the smaller of the two, xmax to be the larger. This should fix the strange plots that commands like

plot (r, xmin=10, xmax=-2).show()

were giving, as I mentioned above.

[jhpalmieri]

new version of 2399 patch, fixing problem when xmin > xmax

[jhpalmieri]

By the way, my patch supersedes Marshall's, but he should get credit for most of the work. (Is it better to have one patch, for easier installation, or two, to make sure the right people get credit for their work?)

One question: in my patch, as I described, if you call plot with arguments xmin=10 and xmax=0, then the plot gets drawn between x=0 and x=10, with no error message. Is this the best behavior, or should an exception be raised instead?

[mhampton]

I thought about that issue a bit (xmin > xmax), and I think the lack of an error message is OK. Thanks for working on this more, I am really swamped with other stuff at the moment.

[jhpalmieri]

Can I give a positive review to mhampton's contribution, and vice versa?

[mabshoff]

Replying to jhpalmieri:

Can I give a positive review to mhampton's contribution, and vice versa?

Yes, that is perfectly fine.

Cheers,

Michael

[mhampton]

My apologies, I should have positively reviewed this before. Now I think the patch has to be rebased. I will try to do that.

[kcrisman]

File 2339-3.2.alpha0.patch is rebased patch against 3.2.alpha0. As JHP says, please give credit to Marshall.

Interestingly, all the plot improvements as of late have already fixed both the Graphics initialization problem of [-1,1] and the problem if you get the range in backwards, i.e. all of the following work with this patch:

plot(r, xmin=10, xmax=-2).show()
plot(r, 10,-2).show()
plot(r, (10,-2)).show()

[kcrisman]

Rebased to 3.2.alpha0

[mhampton]

This is a simple change that seems to solve the problem, positive review.

[mhampton]

By the way, thanks Karl, this had fallen off my radar.

[kcrisman]

No problem; during the school year I don't have much time for new stuff, but I figure I can help out in this way at least!

[mabshoff]

Merged 2339-3.2.alpha0.patch only in Sage 3.2.alpha1