Opened 14 years ago

Closed 13 years ago

Last modified 13 years ago

#5651 closed defect (fixed)

[with patch, positive review] make it so plot(...) passes extra options to show (maybe only those that makes sense)

Reported by: William Stein Owned by: William Cauchois
Priority: major Milestone: sage-4.1.1
Component: graphics Keywords:
Cc: William Cauchois, William Stein, Jason Grout Merged in: sage-4.1.1.alpha0
Authors: Bill Cauchois Reviewers: William Stein, Jason Grout
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

This works now:

plot(sin(x^2),(x,-3,3)).show(figsize=[8,2])

This would be nice:

plot(sin(x^2),(x,-3,3),figsize=[2,8])

The analogue of the above works systematically everywhere for 3d plotting.

Attachments (2)

trac5651.patch (20.7 KB) - added by William Cauchois 14 years ago.
trac5651-rebase.patch (22.5 KB) - added by William Cauchois 13 years ago.
based on sage 4.1.alpha2 (fixed, whoops)

Download all attachments as: .zip

Change History (20)

comment:1 Changed 14 years ago by William Cauchois

Summary: make it so plot(...) passes extra options to show (maybe only those that makes sense)[with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense)

comment:2 Changed 14 years ago by William Stein

Summary: [with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense)[with patch, needs work] make it so plot(...) passes extra options to show (maybe only those that makes sense)

This is really awesome. However, things like this should work too:

sage: line([(0,1), (3,4)],figsize=[10,2])
Traceback (most recent call last):
...
RuntimeError: Error in line(): option 'figsize' not valid.

Also, this should have gridlines:

plot(sin(x^2),(x,-3,3),gridlines=True) + plot(cos(x^2),(x,-3,3))

Clarify the comment

# If you add parameters to show(), you should update the code below. 

}}}

comment:3 Changed 14 years ago by William Cauchois

Summary: [with patch, needs work] make it so plot(...) passes extra options to show (maybe only those that makes sense)[with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense)

Now it works for everything, ever!! With doctests too.

comment:4 Changed 14 years ago by William Stein

Summary: [with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense)[with patch, needs work] make it so plot(...) passes extra options to show (maybe only those that makes sense)

Positive review modulo making so consistent with 3d plotting:

sphere((0,0,0),1, aspect_ratio=[1,4,7]) + sphere((1,1,1),1, aspect_ratio=[1,1,1])

Note that it is the *rightmost* thing that has precedence. Otherwise positive review.

Changed 14 years ago by William Cauchois

Attachment: trac5651.patch added

comment:5 Changed 14 years ago by William Cauchois

Cc: William Stein added

William, I fixed the precedence issue.

comment:6 Changed 14 years ago by William Stein

Summary: [with patch, needs work] make it so plot(...) passes extra options to show (maybe only those that makes sense)[with patch, positive review] make it so plot(...) passes extra options to show (maybe only those that makes sense)

comment:7 Changed 14 years ago by Michael Abshoff

Summary: [with patch, positive review] make it so plot(...) passes extra options to show (maybe only those that makes sense)[with patch, positive review, needs rebase] make it so plot(...) passes extra options to show (maybe only those that makes sense)

This one needs to be rebased due to a doctest merge conflict in arrow.py. On top of that: this is a diff, so please make sure you post a proper patch this time.

mabshoff@sage:/scratch/mabshoff/sage-4.0.alpha0/devel/sage$ patch -p1 --dry-run < trac_5651.patch 
patching file sage/plot/arrow.py
Hunk #1 FAILED at 310.
1 out of 1 hunk FAILED -- saving rejects to file sage/plot/arrow.py.rej
patching file sage/plot/bar_chart.py
patching file sage/plot/bezier_path.py
<SNIP>

Note that various doctest patches in plot/* are going into 4.0.alpha0, so please rebase post 4.0.a0.

Cheers,

Michael

comment:8 Changed 14 years ago by Michael Abshoff

Note that with #6006, #6023 and #6030 applied there are two more small rejects.

Cheers,

Michael

comment:9 Changed 14 years ago by William Cauchois

Summary: [with patch, positive review, needs rebase] make it so plot(...) passes extra options to show (maybe only those that makes sense)[with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense)

I rebased the patch on Sage 4.0.rc0. As far as I could tell, the doctest failures I encountered were not related to this patch. My apologies for posting a diff beforehand, I'm still learning the idiosyncracies of Mercurial Queues :).

comment:10 Changed 14 years ago by Jason Grout

Cc: Jason Grout added

comment:11 Changed 14 years ago by William Stein

Summary: [with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense)[with patch, positive review] make it so plot(...) passes extra options to show (maybe only those that makes sense)

comment:12 Changed 14 years ago by Mike Hansen

Summary: [with patch, positive review] make it so plot(...) passes extra options to show (maybe only those that makes sense)[with patch, needs rebase] make it so plot(...) passes extra options to show (maybe only those that makes sense)

Hi Bill,

I tried applying this to 4.0 and got a bunch of failures. I can probably rebase it later this evening.

comment:13 Changed 14 years ago by William Cauchois

I would appreciate that mhansen, thanks!

comment:14 Changed 13 years ago by William Cauchois

Summary: [with patch, needs rebase] make it so plot(...) passes extra options to show (maybe only those that makes sense)[with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense)

I added a new rebase, so if someone could review this then we can finally get this functionality into Sage.

I feel like I did a little bit of a dirty thing with the new linkmode parameter, which was added somewhere along the line. linkmode is intended to be consumed by show() before the keywords are passed onto save(), so I just popped it from the keyword dict at the beginning of the function.

Changed 13 years ago by William Cauchois

Attachment: trac5651-rebase.patch added

based on sage 4.1.alpha2 (fixed, whoops)

comment:15 Changed 13 years ago by Jason Grout

Reviewers: William Stein, Jason Grout,
Summary: [with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense)[with patch, positive review] make it so plot(...) passes extra options to show (maybe only those that makes sense)

The rebased patch applies fine to my 4.1 tree. I tried a few examples and ran the doctests in plot/*.py[x] and plot/plot3d/*.py[x] and everything seems fine. I'm giving a positive review to the rebasing. That combined with William's almost positive review above adds up to a positive review.

I also looked at the code and it looked reasonable.

Thanks and good work!

comment:16 Changed 13 years ago by Minh Van Nguyen

Authors: Bill Cauchois
Merged in: sage-4.1.1.alpha0
Resolution: fixed
Status: newclosed

Merged trac5651-rebase.patch. That rebased patch contains doctrings that doesn't conform with conventions for formatting docstrings. In particular, in sage/plot/bar_chart.py:

131	    Extra options will get passed on to show(), as long as they are valid:

In sage/plot/bezier_path.py:

171	    Extra options will get passed on to show(), as long as they are valid:

In sage/plot/matrix_plot.py:

58	    Extra options will get passed on to show(), as long as they are valid:
62	    Extra options will get passed on to show(), as long as they are valid:

In sage/plot/polygon.py:

255	    Extra options will get passed on to show(), as long as they are valid:

These issues should be addressed in another enhancement ticket.

comment:17 Changed 13 years ago by Minh Van Nguyen

See #6573 for an enhancement ticket.

comment:18 Changed 13 years ago by Minh Van Nguyen

Reviewers: William Stein, Jason Grout,William Stein, Jason Grout
Note: See TracTickets for help on using tickets.