Ticket #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: | was | Owned by: | wcauchois |
|---|---|---|---|
| Priority: | major | Milestone: | sage-4.1.1 |
| Component: | graphics | Keywords: | |
| Cc: | wcauchois, was, jason | Work issues: | |
| Report Upstream: | Reviewers: | William Stein, Jason Grout | |
| Authors: | Bill Cauchois | Merged in: | sage-4.1.1.alpha0 |
| Dependencies: | Stopgaps: |
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
Change History
comment:1 Changed 4 years ago by wcauchois
- Summary changed from make it so plot(...) passes extra options to show (maybe only those that makes sense) to [with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense)
comment:2 Changed 4 years ago by was
- Summary changed from [with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense) to [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 4 years ago by wcauchois
- Summary changed from [with patch, needs work] make it so plot(...) passes extra options to show (maybe only those that makes sense) to [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 4 years ago by was
- Summary changed from [with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense) to [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.
comment:6 Changed 4 years ago by was
- Summary changed from [with patch, needs work] make it so plot(...) passes extra options to show (maybe only those that makes sense) to [with patch, positive review] make it so plot(...) passes extra options to show (maybe only those that makes sense)
comment:7 Changed 4 years ago by mabshoff
- Summary changed from [with patch, positive review] make it so plot(...) passes extra options to show (maybe only those that makes sense) to [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:9 Changed 4 years ago by wcauchois
- Summary changed from [with patch, positive review, needs rebase] make it so plot(...) passes extra options to show (maybe only those that makes sense) to [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:11 Changed 4 years ago by was
- Summary changed from [with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense) to [with patch, positive review] make it so plot(...) passes extra options to show (maybe only those that makes sense)
comment:12 Changed 4 years ago by mhansen
- Summary changed from [with patch, positive review] make it so plot(...) passes extra options to show (maybe only those that makes sense) to [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 4 years ago by wcauchois
I would appreciate that mhansen, thanks!
comment:14 Changed 4 years ago by wcauchois
- Summary changed from [with patch, needs rebase] make it so plot(...) passes extra options to show (maybe only those that makes sense) to [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 4 years ago by wcauchois
-
attachment
trac5651-rebase.patch
added
based on sage 4.1.alpha2 (fixed, whoops)
comment:15 Changed 4 years ago by jason
- Reviewers set to William Stein, Jason Grout,
- Summary changed from [with patch, needs review] make it so plot(...) passes extra options to show (maybe only those that makes sense) to [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 4 years ago by mvngu
- Status changed from new to closed
- Resolution set to fixed
- Merged in set to sage-4.1.1.alpha0
- Authors set to Bill Cauchois
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 4 years ago by mvngu
See #6573 for an enhancement ticket.
comment:18 Changed 4 years ago by mvngu
- Reviewers changed from William Stein, Jason Grout, to William Stein, Jason Grout
