Sage: Ticket #4529: Implement plots with logarithmic scale
https://trac.sagemath.org/ticket/4529
<p>
Attached is a patch which introduces log scale to <code>Graphics()</code> class.
</p>
<p>
Depends on <a class="closed ticket" href="https://trac.sagemath.org/ticket/12974" title="enhancement: make Graphics class inheritable and some clean ups (closed: fixed)">#12974</a>.
</p>
<p>
Apply
</p>
<ul><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-add-log-scale.patch" title="Attachment 'trac_4529-add-log-scale.patch' in Ticket #4529">trac_4529-add-log-scale.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-add-log-scale.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-single-tick.patch" title="Attachment 'trac_4529-single-tick.patch' in Ticket #4529">trac_4529-single-tick.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-single-tick.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-docs-and-funcs.patch" title="Attachment 'trac_4529-docs-and-funcs.patch' in Ticket #4529">trac_4529-docs-and-funcs.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-docs-and-funcs.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-more-doc.patch" title="Attachment 'trac_4529-more-doc.patch' in Ticket #4529">trac_4529-more-doc.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-more-doc.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-typo_fixes-rebase.patch" title="Attachment 'trac_4529-typo_fixes-rebase.patch' in Ticket #4529">trac_4529-typo_fixes-rebase.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-typo_fixes-rebase.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-final-fixes.patch" title="Attachment 'trac_4529-final-fixes.patch' in Ticket #4529">trac_4529-final-fixes.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-final-fixes.patch" title="Download"></a>
</li></ul><p>
Alternate instructions:
</p>
<p>
Apply the following patches in the specified order. <code>SAGE_ROOT</code> is the directory where the sage installation is present.
</p>
<pre class="wiki">cd SAGE_ROOT/devel/sage
../../sage -hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12974/trac_12974-fix_graphics_attributes-rebase.patch
../../sage -hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12974/trac_12974-refactor.patch
../../sage -hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12974/trac_12974-reorder_some_arguments.patch
../../sage -hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12974/trac_12974-whitespace-rebase.patch
../../sage -hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/4529/trac_4529-add-log-scale.patch
../../sage -hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/4529/trac_4529-single-tick.patch
../../sage -hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/4529/trac_4529-docs-and-funcs.patch
../../sage -hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/4529/trac_4529-more-doc.patch
../../sage -hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/4529/trac_4529-typo_fixes-rebase.patch
../../sage -hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/4529/trac_4529-final_fixes.patch
../../sage -b
</pre><hr />
<p>
<strong>OLD DISCUSSION BELOW :)</strong>
</p>
<p>
Currently plot() has no option to use logarithmic scales.
</p>
<p>
One workaround is to use matplotlib directly, with its semilogy(), semilogx() and loglog() functions, but that wouldn't produce plots with the customisations implemented in sage.
Another workaround is messing with the plot figure like:
</p>
<div class="wiki-code"><div class="code"><pre><span class="kn">import</span> <span class="nn">pylab</span>
p<span class="o">=</span>plot<span class="p">(</span>x<span class="p">,</span>marker<span class="o">=</span><span class="s">'.'</span><span class="p">)</span>
f<span class="o">=</span>pylab<span class="o">.</span>figure<span class="p">()</span>
f<span class="o">.</span>gca<span class="p">()</span><span class="o">.</span>set_xscale<span class="p">(</span><span class="s">'log'</span><span class="p">)</span>
p<span class="o">.</span>save<span class="p">(</span>figure<span class="o">=</span>f<span class="p">)</span>
</pre></div></div><p>
But that creates two problems:
</p>
<ul><li>The first problem is that the adaptive choosing of points just considers linear scale, so the points get too much spaced apart in the beginning of the plot and too close in the end.
</li><li>The second problem relates to the axis, which, for the same reason, isn't located right.
</li></ul><p>
Also, this requires the user to know how to deal with figures, which is not directly exposed by sage.
</p>
<p>
There are some possibilities to fix that:
</p>
<ol><li>Make plot() detect if the figure changes the scales and modify the adaptive algorithm and the axis codes accordingly
</li><li>Create a kwarg to tell plot() to implement the scale-change internally
</li><li>Create other functions to use loglog(), semilogx() and semilogy()
</li><li>Many (or all) of the above together, since they aren't mutually exclusive
</li></ol><p>
From what I noticed, Mathematica implements the separate functions way, but it may be better to fix the issue in plot() itself and if the other functions are wanted, just make it so that they call plot() with the correct arguments
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/4529
Trac 1.1.6ronanpaixaoSat, 15 Nov 2008 18:59:49 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>sage0.png</em>
</li>
</ul>
<p>
"Wrong" sage plot with log scale
</p>
TicketmabshoffSat, 15 Nov 2008 19:03:30 GMTstatus, milestone changed; resolution set
https://trac.sagemath.org/ticket/4529#comment:1
https://trac.sagemath.org/ticket/4529#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>duplicate</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage-wishlist</em> to <em>sage-duplicate/invalid/wontfix</em>
</li>
</ul>
<p>
This is a dupe if <a class="closed ticket" href="https://trac.sagemath.org/ticket/4530" title="enhancement: Implement plots with logarithmic scale (closed: duplicate)">#4530</a>
</p>
<p>
Cheers,
</p>
<p>
Michael
</p>
TicketmabshoffSat, 15 Nov 2008 19:04:04 GMTstatus changed; resolution deleted
https://trac.sagemath.org/ticket/4529#comment:2
https://trac.sagemath.org/ticket/4529#comment:2
<ul>
<li><strong>status</strong>
changed from <em>closed</em> to <em>reopened</em>
</li>
<li><strong>resolution</strong>
<em>duplicate</em> deleted
</li>
</ul>
TicketmabshoffSat, 15 Nov 2008 19:04:25 GMTowner, status changed
https://trac.sagemath.org/ticket/4529#comment:3
https://trac.sagemath.org/ticket/4529#comment:3
<ul>
<li><strong>owner</strong>
changed from <em>somebody</em> to <em>ronanpaixao</em>
</li>
<li><strong>status</strong>
changed from <em>reopened</em> to <em>new</em>
</li>
</ul>
TicketmabshoffSat, 15 Nov 2008 19:04:33 GMTmilestone changed
https://trac.sagemath.org/ticket/4529#comment:4
https://trac.sagemath.org/ticket/4529#comment:4
<ul>
<li><strong>milestone</strong>
changed from <em>sage-duplicate/invalid/wontfix</em> to <em>sage-3.2.1</em>
</li>
</ul>
TicketjasonThu, 12 Nov 2009 04:13:04 GMT
https://trac.sagemath.org/ticket/4529#comment:5
https://trac.sagemath.org/ticket/4529#comment:5
<p>
See <a class="closed ticket" href="https://trac.sagemath.org/ticket/1431" title="enhancement: basic plotting: add support for setting the location and labels of all ... (closed: fixed)">#1431</a> for a way to solve this.
</p>
TicketjasonThu, 13 Oct 2011 13:57:32 GMTupstream set
https://trac.sagemath.org/ticket/4529#comment:6
https://trac.sagemath.org/ticket/4529#comment:6
<ul>
<li><strong>upstream</strong>
set to <em>N/A</em>
</li>
</ul>
<p>
Since <a class="closed ticket" href="https://trac.sagemath.org/ticket/1431" title="enhancement: basic plotting: add support for setting the location and labels of all ... (closed: fixed)">#1431</a> ended up just being about ticks (not the scale), <a class="closed ticket" href="https://trac.sagemath.org/ticket/1431" title="enhancement: basic plotting: add support for setting the location and labels of all ... (closed: fixed)">#1431</a> doesn't directly address how to change the scale of the plot.
</p>
TicketjasonThu, 13 Oct 2011 14:32:12 GMT
https://trac.sagemath.org/ticket/4529#comment:7
https://trac.sagemath.org/ticket/4529#comment:7
<p>
Here are some comments about an API. How about adding a show keyword that specifies the scale of the axes.
</p>
<p>
<code>scale='log'</code> -- a string specifies the scale of the vertical axis
</p>
<p>
<code>scale=('log','linear')</code> -- a tuple or list of two strings specifies scales for both axes
</p>
<p>
We'd probably like some way to pass in arguments to the scale, since different scales have different options. This looks ugly: <code>scale=( ('log', {'base': 2}), 'linear')</code>
</p>
TicketjasonThu, 13 Oct 2011 14:33:59 GMT
https://trac.sagemath.org/ticket/4529#comment:8
https://trac.sagemath.org/ticket/4529#comment:8
<p>
We could also do something like <code>xscale='log'</code> or <code>xscale=('log',{'base': 2})</code> and similarly for yscale. I don't like using x and y, though, since the variables in the plot might not be x and y.
</p>
TicketkcrismanThu, 13 Oct 2011 15:42:07 GMT
https://trac.sagemath.org/ticket/4529#comment:9
https://trac.sagemath.org/ticket/4529#comment:9
<p>
My sense is that the API should look like the tick marks API.
</p>
<p>
Here are some comments Jason made on sage-support about this.
</p>
<pre class="wiki">> To change the scale, you can modify the plot afterwards, but I am
> running into some sort of problem doing it:
> sage: p=plot(e^x,(x,0,10))
> sage: m=p.matplotlib()
> sage: from matplotlib.backends.backend_agg import FigureCanvasAgg
> sage: m.set_canvas(FigureCanvasAgg(m))
> sage: m.gca().set_yscale('log')
> sage: m.savefig('test.png')
It seems something was wrong with the plot in the above example, or
something. Anyways, starting with:
p=plot(x,(x,1,10))
works fine.
To do #4529, I'd suggest adding a keyword to show that defines the
scales of the x and y axes. I've added some comments to the ticket.
</pre><hr />
<p>
How was I not cc:ed on this ticket before? ;-)
</p>
TicketkcrismanThu, 13 Oct 2011 15:44:03 GMT
https://trac.sagemath.org/ticket/4529#comment:10
https://trac.sagemath.org/ticket/4529#comment:10
<p>
Also, <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/5128" title="enhancement: [with patch, needs work] matplotlib Graphics() wrapper (needs_work)">#5128</a> would appear to be slightly related.
</p>
TicketkcrismanThu, 13 Oct 2011 15:48:53 GMT
https://trac.sagemath.org/ticket/4529#comment:11
https://trac.sagemath.org/ticket/4529#comment:11
<p>
The error with <code>e^x</code> is
</p>
<pre class="wiki">MaskError: Cannot convert masked element to a Python int.
</pre><p>
but seems to be related to there being something other than linearity involved. Linear functions work, anything with <code>^</code> or <code>**</code> or <code>sin</code> doesn't.
</p>
<pre class="wiki">--> 154 self._renderer.draw_text_image(font.get_image(), int(x), int(y) + 1, angle, gc)
</pre><p>
is the problem - it's converting one of the elements, which is supposed to be skipped (masked, right?) to an int.
</p>
TicketppurkaWed, 08 Feb 2012 12:44:26 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>logplots.py</em>
</li>
</ul>
<p>
plot in logarithmic scale.
</p>
TicketppurkaWed, 08 Feb 2012 12:50:17 GMT
https://trac.sagemath.org/ticket/4529#comment:12
https://trac.sagemath.org/ticket/4529#comment:12
<p>
<a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/logplots.py" title="Attachment 'logplots.py' in Ticket #4529">logplots.py</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/logplots.py" title="Download"></a> has a new class <code>LogGraphics</code> that I implemented and have been using for the past few months. Integrating it with Graphics seemed quite a painful process, so I had to go this direction and make my own class. Currently, it handles many <em>but not all</em> of the arguments that the <code>Graphics</code> class supports. In addition it uses <code>matplotlib.plt</code> to do the log plot; otherwise I ran into all sorts of problems with matplotlib (like the ones mentioned in earlier comments).
</p>
<p>
In engineering, we often need logarithmic plots and the logarithmic plots sometimes is of the form that the x-axis <em>decreases</em> as we go towards the right (for example if we plot decreasing probabilities on the x-axis). This <code>LogGraphics</code> takes this into account and makes sure that if a list of x-axis points with decreasing values along the higher indices of the list, then it plots the graph with a decreasing x-axis.
</p>
TicketppurkaWed, 08 Feb 2012 13:02:12 GMT
https://trac.sagemath.org/ticket/4529#comment:13
https://trac.sagemath.org/ticket/4529#comment:13
<p>
Sorry, I meant <code>matplotlib.pyplot</code> in the above comment.
</p>
TicketeviatarbachWed, 18 Apr 2012 02:15:57 GMTstatus changed
https://trac.sagemath.org/ticket/4529#comment:14
https://trac.sagemath.org/ticket/4529#comment:14
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
TicketppurkaWed, 18 Apr 2012 06:37:11 GMT
https://trac.sagemath.org/ticket/4529#comment:15
https://trac.sagemath.org/ticket/4529#comment:15
<p>
I am not sure if this needs to be set to "needs_review". The main thing it is lacking is that it doesn't inherit the <code>Graphics</code> class, and hence the set of plot options it supports is much less.
</p>
<p>
On the other hand, I did try to make it inherit the <code>Graphics</code> class but then I ran into a big hurdle: the variables in the <code>Graphics</code> class are defined with double underscore <code>__</code> and so even after I inherit it, I need to use (IMHO ugly) setters and getters in order to access those variables. I tried to overcome this limitation by inheriting <code>Graphics</code> in the class <code>LogGraphics</code> and defining a separate (and mostly identical) <code>__init__</code> in <code>LogGraphics</code> but then the methods wouldn't work. Since I needed to rewrite almost everything, I decided to just rewrite everything from scratch.
</p>
<p>
One thing that I plan to do is change all the variables in the <code>Graphics</code> class to be defined with a single <code>_</code> and see how it works out. Perhaps then it might be possible to integrate this patch better and consequently have access to all the methods (and hence plot options) available to the <code>plot</code> command.
</p>
TicketkcrismanWed, 18 Apr 2012 14:21:25 GMT
https://trac.sagemath.org/ticket/4529#comment:16
https://trac.sagemath.org/ticket/4529#comment:16
<p>
Hmm, that's odd that you had to do this. Here are two "Sage-ic" ideas.
</p>
<ul><li>Have it inherit from <code>GraphicPrimitive</code>. That is how most classes are done, and hopefully (?) would work.
</li><li>Have this be a <code>show</code> option, or something like that. In principle, we wouldn't even want the graphic itself to be plotted logarithmically; one could imagine wanting to have a <code>Line</code> or other plot and then to view it with log, semilog, reverse log, or regular scales. See Jason's wise <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:7" title="Comment 7">comment:7</a>.
</li></ul>
TicketppurkaWed, 18 Apr 2012 15:29:49 GMT
https://trac.sagemath.org/ticket/4529#comment:17
https://trac.sagemath.org/ticket/4529#comment:17
<ul><li><code>GraphicsPrimitive</code> is unfortunately too crude for most purposes. All the important functions are defined in <code>Graphics</code>.
</li><li>I did try using <code>_render_on_subplot</code> from <code>line.py</code> but ran into the same problem as you described in <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:9" title="Comment 9">comment:9</a>. It simply doesn't work since matplotlib fails to re-render the line in log scale. Actually I ran into many more problems; I just can't remember what else. The most painless method seemed to be to use pyplot (or even pylab, but we can avoid importing that since we don't need numpy).
</li></ul><p>
That said, it is IMHO better to have logarithmic plots as a separate class. For instance, it doesn't make much sense to "add" plots with different scalings (and I also raise an error in the class I created).
</p>
TicketkiniWed, 16 May 2012 14:09:42 GMTstatus changed
https://trac.sagemath.org/ticket/4529#comment:18
https://trac.sagemath.org/ticket/4529#comment:18
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
I'm currently cleaning up tickets marked needs_review which have no patches attached, which includes this one, so back to needs_work this goes.
</p>
TicketppurkaWed, 16 May 2012 16:20:32 GMT
https://trac.sagemath.org/ticket/4529#comment:19
https://trac.sagemath.org/ticket/4529#comment:19
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:18" title="Comment 18">kini</a>:
</p>
<blockquote class="citation">
<p>
I'm currently cleaning up tickets marked needs_review which have no patches attached, which includes this one, so back to needs_work this goes.
</p>
</blockquote>
<p>
That's fine. I am actually working on a patch which
</p>
<ol><li>modifies the <code>Graphics</code> class to have all the attributes start with a single underscore <code>._</code> instead of <code>.__</code>. This is already working and passes all doctects at least in <code>devel/sage/sage/plot</code>
</li><li>inherits the <code>Graphics</code> class and introduces logarithmic plots in a separate file. This is in progress and I hope I have a patch to attach to this ticket soon.
</li></ol>
TicketjasonWed, 16 May 2012 16:33:37 GMT
https://trac.sagemath.org/ticket/4529#comment:20
https://trac.sagemath.org/ticket/4529#comment:20
<p>
Awesome. Just yesterday I had a feature request for log-log and semilog plots!
</p>
TicketppurkaSat, 19 May 2012 13:12:51 GMTdescription changed; dependencies set
https://trac.sagemath.org/ticket/4529#comment:21
https://trac.sagemath.org/ticket/4529#comment:21
<ul>
<li><strong>dependencies</strong>
set to <em>12974</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=21">diff</a>)
</li>
</ul>
<p>
I added a patch to <code>Graphics</code> class which introduces log plots. Some salient points
</p>
<ol><li>I had to "disable" some tick formatting for log plots because matplotlib wasn't behaving well with the formatting that is done in <code>Graphics().maptplotlib()</code> (ex. the error in <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:11" title="Comment 11">comment:11</a>, out of memory error, etc)
</li><li>The patch in this ticket relies on the patches in <a class="closed ticket" href="https://trac.sagemath.org/ticket/12974" title="enhancement: make Graphics class inheritable and some clean ups (closed: fixed)">#12974</a> which is mostly a cleanup of the <code>Graphics</code> class.
</li><li>In trying to implement my own class, I started to look at each of the matplotlib functions more carefully, and found out the reason(s) why setting the scale wasn't working (see point 1.). The result is that I could implement log scale right inside <code>Graphics</code> by carefully weeding out the corner cases. I hope I got all the corner cases.
</li></ol><p>
Todo:
</p>
<ol><li>A patch to <code>plot()</code> and other functions will take more time to implement. :(
</li><li>Probably need to make sure that user does not specify tick formatters and locators which don't behave well with log plots.
</li><li>Feedback is welcome! I need to know if I missed something.
</li></ol><p>
Example code:
</p>
<pre class="wiki">p = plot(exp, 1, 10)
p.set_scale('loglog')
p.show()
xd=range(-5,5); yd=[10**_ for _ in xd]; p=list_plot(zip(xd, yd),plotjoined=True)
p.set_yscale('log', 2) # Set only y-axis to log and with base of log being 2.
p.show()
</pre>
TicketppurkaSat, 19 May 2012 16:07:50 GMT
https://trac.sagemath.org/ticket/4529#comment:22
https://trac.sagemath.org/ticket/4529#comment:22
<p>
Hmm.. there is still a problem if I modify the <code>Graphics</code> class. It becomes impossible to add 2D and 3D graphics.
</p>
TicketppurkaSun, 20 May 2012 10:59:36 GMT
https://trac.sagemath.org/ticket/4529#comment:23
https://trac.sagemath.org/ticket/4529#comment:23
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:22" title="Comment 22">ppurka</a>:
</p>
<blockquote class="citation">
<p>
Hmm.. there is still a problem if I modify the <code>Graphics</code> class. It becomes impossible to add 2D and 3D graphics.
</p>
</blockquote>
<p>
It was a silly thing. I just needed to reorder the check for <code>._*scale</code> to after the check for <code>Graphics3d</code> in <code>__add__()</code>. The updated patch now passes all doctests in <code>sage/plot</code>! Also, <code>SHOW_OPTIONS, matplotlib()</code> have two extra arguments: <code>scale</code>, <code>base</code> which are identical in behavior to the arguments in <code>set_scale()</code>. So, now it is possible to do this:
</p>
<pre class="wiki">p = plot(exp, 1, 10)
p.show(scale=('loglog', 2))
</pre>
TicketppurkaSun, 20 May 2012 11:00:00 GMTdependencies changed
https://trac.sagemath.org/ticket/4529#comment:24
https://trac.sagemath.org/ticket/4529#comment:24
<ul>
<li><strong>dependencies</strong>
changed from <em>12974</em> to <em>#12974</em>
</li>
</ul>
TicketppurkaSun, 20 May 2012 17:13:48 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-add_logscale_to_Graphics.patch</em>
</li>
</ul>
<p>
Apply to devel/sage
</p>
TicketppurkaSun, 20 May 2012 17:14:13 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-add_docs_eg_to_some_user_facing_functions.patch</em>
</li>
</ul>
<p>
Apply to devel/sage
</p>
TicketppurkaSun, 20 May 2012 17:17:55 GMTstatus, description changed
https://trac.sagemath.org/ticket/4529#comment:25
https://trac.sagemath.org/ticket/4529#comment:25
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=25">diff</a>)
</li>
</ul>
<p>
Added <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-add_docs_eg_to_some_user_facing_functions.patch" title="Attachment 'trac_4529-add_docs_eg_to_some_user_facing_functions.patch' in Ticket #4529">another patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-add_docs_eg_to_some_user_facing_functions.patch" title="Download"></a> to other user facing functions. Actually most of the functions in <code>sage.plot.*</code> work when <code>scale=...</code> is passed as an argument while calling the function. Using the log scale makes sense in only a couple of them though, and I have added some documentation and examples for the cases I think are pertinent.
</p>
<p>
Finally, I added some extra functions <code>{loglog,semilogx,semilogy}_plot</code>, and the corresponding list_plots. I think this should undergo a review now.
</p>
<p>
These set of patches passes all doctests in <code>devel/sage/sage/plot</code>.
</p>
TicketkcrismanThu, 24 May 2012 22:39:02 GMTdescription changed; author set
https://trac.sagemath.org/ticket/4529#comment:26
https://trac.sagemath.org/ticket/4529#comment:26
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=26">diff</a>)
</li>
<li><strong>author</strong>
set to <em>Punarbasu Purkayastha</em>
</li>
</ul>
<p>
Patchbot: Apply <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-add_logscale_to_Graphics.patch" title="Attachment 'trac_4529-add_logscale_to_Graphics.patch' in Ticket #4529">trac_4529-add_logscale_to_Graphics.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-add_logscale_to_Graphics.patch" title="Download"></a> and <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-add_docs_eg_to_some_user_facing_functions.patch" title="Attachment 'trac_4529-add_docs_eg_to_some_user_facing_functions.patch' in Ticket #4529">trac_4529-add_docs_eg_to_some_user_facing_functions.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-add_docs_eg_to_some_user_facing_functions.patch" title="Download"></a>.
</p>
TicketkcrismanThu, 24 May 2012 22:51:38 GMT
https://trac.sagemath.org/ticket/4529#comment:27
https://trac.sagemath.org/ticket/4529#comment:27
<p>
I hope to be able to go over this very valuable idea at the current Bug Days. Trivial comment while I'm doing a cursory read-through
</p>
<pre class="wiki">- ``linear`` -- both the axes are linear.
</pre><p>
should probably indicate
</p>
<pre class="wiki">- ``'linear'`` -- both the axes are linear.
</pre><p>
or
</p>
<pre class="wiki">- 'linear' -- both the axes are linear.
</pre><p>
and similarly in all other cases, especially when looking at inputs (since it's very important that these are strings, not just commands.
</p>
<hr />
<p>
Comments:
</p>
<ul><li>I feel like it would be good to have a little discussion about whether the scale should be "hardcoded" into a Graphics object; somehow this feels not right to me, though I'd hate to ignore the work here for that reason. It just seems better to be able to add plots, then "show" them however we want. Naturally, since a lot of that is set as keywords passed from plot to show, there could be conflicts, but that could be up to the user.
</li></ul><blockquote>
<p>
Especially since the bulk of the "work" done in the code still happens in <code>show</code> and friends, I don't see why we couldn't just cherry-pick the keyword <code>scale</code> and handle it like we do things like plot tick formatting, separately from <code>Graphics</code>. The interface is cleaner that way.
</p>
</blockquote>
<ul><li>Separately, in either case, should we globally import all of these many new functions? For instance, having the <code>listplot</code> variants and <code>semilogx_list_plot, semilogy_list_plot</code> both available (as opposed to <code>semilog_plot(keywords=x or y)</code>) seems overkill.
</li></ul>
TicketppurkaThu, 24 May 2012 23:45:00 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/4529#comment:28
https://trac.sagemath.org/ticket/4529#comment:28
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>doctests, strings</em>
</li>
</ul>
<p>
It just seemed easier and cleaner to handle the scaling by hardcoding the scale. I will see how the patch turns out if I don't hardcode it.
</p>
<p>
Most people will expect the functions <code>semilog*</code> and <code>loglog*</code> to be present. For instance, matlab has all of those commands (matlab has it only for list plots, the function plotter called <code>ezplot</code> does not have it AFAIK), and mathematica has <code>LogLogPlot</code> and <code>LogLinearPlot</code>.
</p>
<p>
If it is not desirable, then we can simply have just two functions <code>log_plot(scale='loglog'|'semilogx'|'semilogy', funcs, ...)</code>, and <code>log_list_plot(scale='loglog'|'semilogx'|'semilogy', data, ...)</code>.
</p>
TicketjasonFri, 25 May 2012 00:40:00 GMT
https://trac.sagemath.org/ticket/4529#comment:29
https://trac.sagemath.org/ticket/4529#comment:29
<p>
+1 to having loglog and semilog convenience functions.
</p>
TicketkcrismanFri, 25 May 2012 00:43:51 GMT
https://trac.sagemath.org/ticket/4529#comment:30
https://trac.sagemath.org/ticket/4529#comment:30
<p>
I think that anything that roughly approximates Mathematica and Matlab is fine, it was just having so many of them that might be a problem. <code>log_plot</code> and <code>semilog_plot(axis=foo)</code> or something better would be good, and perhaps a couple similar <code>list_plot</code> ones... I have to admit I always just use <code>points()</code> instead of <code>list_plot</code>, is <code>list_plot</code> used as the name in one of these programs?
</p>
<p>
Jason, how <strong>many</strong> would be useful? It's the <code>semilogx</code> and <code>semilogy</code> that seems a bit much, though if it's standard in other programs I guess it would be ok.
</p>
TicketjasonFri, 25 May 2012 00:49:23 GMT
https://trac.sagemath.org/ticket/4529#comment:31
https://trac.sagemath.org/ticket/4529#comment:31
<p>
I personally would say a loglog and semilog (defaulting to semilogy) would be good, with an option to switch the semilog to x or y. I guess a list plot would be convenient too, though I agree with you that points() or line() in general should be used over list_plot. They are more powerful (mostly) anyway.
</p>
TicketppurkaFri, 25 May 2012 08:07:03 GMT
https://trac.sagemath.org/ticket/4529#comment:32
https://trac.sagemath.org/ticket/4529#comment:32
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:31" title="Comment 31">jason</a>:
</p>
<blockquote class="citation">
<p>
I personally would say a loglog and semilog (defaulting to semilogy) would be good, with an option to switch the semilog to x or y. I guess a list plot would be convenient too, though I agree with you that points() or line() in general should be used over list_plot. They are more powerful (mostly) anyway.
</p>
</blockquote>
<p>
For <em>consistency</em>, we should have just one convention. It is very confusing if the options of <code>plot</code> (except for probably <code>plotjoined</code> and <code>data</code>) are also valid options for <code>list_plot</code>, but then we introduce an inconsistency via log plots. So, I would be in favor of either
</p>
<ol><li>Don't have any of the <code>loglog_*, semilog*</code> and handle scaling only through the <code>scale</code> and <code>base</code> parameters of <code>plot</code> and <code>list_plot</code> (and actually all other plots)
</li><li>Have all the functions <code>loglog_plot</code>, <code>loglog_list_plot</code> available, and perhaps change <code>semilog[xy]*</code> to <code>semilog*</code> with an extra optional argument <code>log_axis='x'/'y'</code>. In case we follow this second rule, I would like this extra argument to be different from <code>axis</code> because it can be confused with <code>axes=True/False</code>.
</li></ol><p>
I would really like this issue to be sorted out first.
</p>
TicketkcrismanFri, 25 May 2012 13:59:19 GMT
https://trac.sagemath.org/ticket/4529#comment:33
https://trac.sagemath.org/ticket/4529#comment:33
<blockquote class="citation">
<p>
For <em>consistency</em>, we should have just one convention.
</p>
</blockquote>
<p>
Agreed.
</p>
<blockquote class="citation">
<p>
It is very confusing if the options of <code>plot</code> (except for probably <code>plotjoined</code> and <code>data</code>) are also valid options for <code>list_plot</code>, but then we introduce an inconsistency via log plots. So, I would be in favor of either
</p>
</blockquote>
<p>
How would this introduce an inconsistency? Is the suggestion on the table that the log option would only be for one of them? I don't see why we can't have our cake and eat it too.
</p>
<ul><li>Log options in <code>show</code> or <code>save</code>
</li><li><code>loglog_plot(f,(x,a,b))</code> is an alias for <code>plot(f,(x,a,b),scale=foo)</code>
</li></ul><blockquote class="citation">
<ol><li>Don't have any of the <code>loglog_*, semilog*</code> and handle scaling only through the <code>scale</code> and <code>base</code> parameters of <code>plot</code> and <code>list_plot</code> (and actually all other plots)
</li></ol></blockquote>
<p>
If Mma and friends have it, this is probably not a good idea.
</p>
<blockquote class="citation">
<ol start="2"><li>Have all the functions <code>loglog_plot</code>, <code>loglog_list_plot</code> available, and perhaps change <code>semilog[xy]*</code> to <code>semilog*</code> with an extra optional argument <code>log_axis='x'/'y'</code>. In case we follow this second rule, I would like this extra argument to be different from <code>axis</code> because it can be confused with <code>axes=True/False</code>.
</li></ol></blockquote>
<p>
Yes, that's a very good idea!
</p>
<blockquote class="citation">
<p>
I would really like this issue to be sorted out first.
</p>
</blockquote>
<p>
Agreed. Jason, should we raise this on sage-devel?
</p>
TicketjasonFri, 25 May 2012 17:21:44 GMT
https://trac.sagemath.org/ticket/4529#comment:34
https://trac.sagemath.org/ticket/4529#comment:34
<p>
Sure, let's raise it on sage-devel. Make sure the proposal provides specific options to vote for.
</p>
TicketkcrismanSat, 26 May 2012 05:05:15 GMT
https://trac.sagemath.org/ticket/4529#comment:35
https://trac.sagemath.org/ticket/4529#comment:35
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:34" title="Comment 34">jason</a>:
</p>
<blockquote class="citation">
<p>
Sure, let's raise it on sage-devel. Make sure the proposal provides specific options to vote for.
</p>
</blockquote>
<p>
Okay, hope I did it clearly enough.
</p>
<p>
<a class="ext-link" href="http://groups.google.com/group/sage-devel/browse_thread/thread/af20ea19c09d14a0"><span class="icon"></span>http://groups.google.com/group/sage-devel/browse_thread/thread/af20ea19c09d14a0</a>
</p>
TicketppurkaSat, 26 May 2012 09:07:46 GMT
https://trac.sagemath.org/ticket/4529#comment:36
https://trac.sagemath.org/ticket/4529#comment:36
<p>
Thanks kcrisman. That poll is comprehensive enough.
</p>
<p>
Updated the <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-add_logscale_to_Graphics.2.patch" title="Attachment 'trac_4529-add_logscale_to_Graphics.2.patch' in Ticket #4529">Graphics patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-add_logscale_to_Graphics.2.patch" title="Download"></a>. This now has modifications only to matplotlib and sister functions, and leaves the <code>Graphics</code> class's attributes alone.
</p>
<p>
There was a problem with the ticker in that the labels were in scientific notation (ex. <code>1e10</code>) and not in the <code>base^exponent</code> form (ex. <code>10^10</code>). This is now fixed, except for the case when the user enters a custom tick formatter. This last case is up to the user to handle.
</p>
<p>
The interface to <code>plot</code> and <code>list_plot</code> remains unchanged. However, I will wait for the poll in sage-devel before deciding what extra plot commands to introduce.
</p>
TicketppurkaSat, 26 May 2012 12:00:02 GMTdescription, work_issues changed
https://trac.sagemath.org/ticket/4529#comment:37
https://trac.sagemath.org/ticket/4529#comment:37
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=37">diff</a>)
</li>
<li><strong>work_issues</strong>
changed from <em>doctests, strings</em> to <em>convenience functions</em>
</li>
</ul>
TicketppurkaSat, 26 May 2012 12:08:04 GMT
https://trac.sagemath.org/ticket/4529#comment:38
https://trac.sagemath.org/ticket/4529#comment:38
<p>
Updated to the correct patch. Apparently I uploaded the wrong patch several hours ago.
</p>
TicketkcrismanSat, 26 May 2012 23:53:09 GMTdescription changed; reviewer set
https://trac.sagemath.org/ticket/4529#comment:39
https://trac.sagemath.org/ticket/4529#comment:39
<ul>
<li><strong>reviewer</strong>
set to <em>Karl-Dieter Crisman</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=39">diff</a>)
</li>
</ul>
<p>
Some comments:
</p>
<ul><li>Shouldn't <code>base=2</code> raise an error when <code>scale='linear'</code> in your example? Maybe the
<pre class="wiki"> if scale is None:
return ('linear', 'linear', 10, 10)
</pre>could return 'linear', 'linear', None, None?
</li><li>In <code>_matplotlib_tick_formatter</code>, should <code>base</code> and <code>scale</code> be next to each other in the function definition? (This is a very minor critique, of course.)
</li><li>Nice consolidation of the <code>ticklabels</code> business at the end of the patch.
</li><li>Regardless of the outcome of the poll (on which you can vote), I think one should add a lot more examples in the documentation for <code>show</code> for the various options. Lots of them.
</li><li>What's going on with the <code>pr, i = '', 0</code> thing removed? I just don't know what it had been doing - seems to have been dead code, but I always get nervous when I have no idea what it <em>used</em> to do...
</li><li>kini says that the <code>[13:]</code> seems brittle if matplotlib's API changes; would it be possible to remove the specific string <code>\\mathdefault</code> instead?
</li><li>I wonder about the not setting of the spines outward when the axes shouldn't cross. Here is an example which serves the point:
<pre class="wiki">sage: G = plot(exp(x), (x,5,10))
sage: G.show(scale=('semilogy', 2))
</pre>I don't even think this is a very atypical example to arise in practice. It should be documented somehow.
</li><li>It's fairly easy to have just one tick in a given direction, which usually raises an error in normal plots but isn't raising an error for yours. I'm not sure if one would want to raise an error like "Use a different base so that you get at least two ticks!" or something.
</li></ul><p>
But even with all of these comments, and waiting for the post-poll patch, <strong>fantastic</strong> job on this. Someone had to come along to finally wrap this for us, it's been requested zillions of times, and this is very worth the effort, thank you so much.
</p>
TicketppurkaSun, 27 May 2012 03:38:50 GMT
https://trac.sagemath.org/ticket/4529#comment:40
https://trac.sagemath.org/ticket/4529#comment:40
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:39" title="Comment 39">kcrisman</a>:
</p>
<blockquote class="citation">
<p>
Some comments:
</p>
<ul><li>Shouldn't <code>base=2</code> raise an error when <code>scale='linear'</code> in your example? Maybe the
<pre class="wiki"> if scale is None:
return ('linear', 'linear', 10, 10)
</pre>could return 'linear', 'linear', None, None?
</li></ul></blockquote>
<p>
I had thought about it. My decision was to silently ignore this error because it is not fatal in any way and we handle it properly (i.e. we ignore it and do the right thing).
</p>
<p>
<strong>Edit:</strong> This seems to be the same behavior as in matplotlib.
</p>
<blockquote class="citation">
<ul><li>In <code>_matplotlib_tick_formatter</code>, should <code>base</code> and <code>scale</code> be next to each other in the function definition? (This is a very minor critique, of course.)
</li></ul></blockquote>
<p>
Well, except for <code>subplot</code>, the rest of the arguments are alphabetically arranged. :) Personally, I find it quite hard to find out where a particular function or argument is present in a typical Sage code. There is no particular manner in which the functions are arranged. Especially in several thousand line files like graphics.py it becomes hard to scroll around and edit code.
</p>
<blockquote class="citation">
<ul><li>Regardless of the outcome of the poll (on which you can vote), I think one should add a lot more examples in the documentation for <code>show</code> for the various options. Lots of them.
</li></ul></blockquote>
<p>
I will add some more.
</p>
<blockquote class="citation">
<ul><li>What's going on with the <code>pr, i = '', 0</code> thing removed? I just don't know what it had been doing - seems to have been dead code, but I always get nervous when I have no idea what it <em>used</em> to do...
</li></ul></blockquote>
<p>
Yes. I have no idea what it was for. It is dead code, so I removed it.
</p>
<blockquote class="citation">
<ul><li>kini says that the <code>[13:]</code> seems brittle if matplotlib's API changes; would it be possible to remove the specific string <code>\\mathdefault</code> instead?
</li></ul></blockquote>
<p>
To remove it from matplotlib, we need to set <code>rcParams['text.usetex']=True</code>. But this makes matplotlib try to compile latex on its own and use dvipng to convert from dvi to png, etc. Moreover, this parameter seems to be persistent and remains throughout the current session. So, simply editing the string seemed a more viable option to me.
</p>
<p>
If the API changes (which seems unlikely to me), then the fix will be very easy too.
</p>
<blockquote class="citation">
<ul><li>I wonder about the not setting of the spines outward when the axes shouldn't cross. Here is an example which serves the point:
<pre class="wiki">sage: G = plot(exp(x), (x,5,10))
sage: G.show(scale=('semilogy', 2))
</pre>I don't even think this is a very atypical example to arise in practice. It should be documented somehow.
</li></ul></blockquote>
<p>
I will have to see how to handle this. Messing around with the spines was one of the primary reasons why setting scale wasn't working - the "converting masked to int" error.
</p>
<blockquote class="citation">
<ul><li>It's fairly easy to have just one tick in a given direction, which usually raises an error in normal plots but isn't raising an error for yours. I'm not sure if one would want to raise an error like "Use a different base so that you get at least two ticks!" or something.
</li></ul></blockquote>
<p>
I think it is up to the user to either change their range, or their base, or provide custom ticks.
</p>
<blockquote class="citation">
<p>
But even with all of these comments, and waiting for the post-poll patch, <strong>fantastic</strong> job on this. Someone had to come along to finally wrap this for us, it's been requested zillions of times, and this is very worth the effort, thank you so much.
</p>
</blockquote>
<p>
Thanks. I needed it for my own research! :)
</p>
TicketkiniSun, 27 May 2012 03:45:54 GMT
https://trac.sagemath.org/ticket/4529#comment:41
https://trac.sagemath.org/ticket/4529#comment:41
<p>
Oh, I didn't mean to prevent <code>\\mathdefault</code> from coming into the string at all. I meant to just specifically remove the substring <code>\\mathdefault</code> (say with <code>.replace("\\mathdefault","")</code> or something).
</p>
TicketkcrismanSun, 27 May 2012 04:09:31 GMT
https://trac.sagemath.org/ticket/4529#comment:42
https://trac.sagemath.org/ticket/4529#comment:42
<blockquote class="citation">
<p>
I had thought about it. My decision was to silently ignore this error because it is not fatal in any way and we handle it properly (i.e. we ignore it and do the right thing).
</p>
<p>
<strong>Edit:</strong> This seems to be the same behavior as in matplotlib.
</p>
</blockquote>
<p>
Okay, just asking. Maybe this should be documented (that is, explained that it's ok that no error is raised).
</p>
<blockquote class="citation">
<p>
Well, except for <code>subplot</code>, the rest of the arguments are alphabetically arranged. :) Personally, I find it quite hard to find out where a particular function or argument is present in a typical Sage code. There is no particular manner in which the functions are arranged. Especially in several thousand line files like graphics.py it becomes hard to scroll around and edit code.
</p>
</blockquote>
<p>
Oh yeah, it's REALLY hard to find stuff - you just have to get used to it. Ok.
</p>
<blockquote class="citation">
<p>
I will add some more.
</p>
</blockquote>
<p>
Great.
</p>
<blockquote class="citation">
<p>
Yes. I have no idea what it was for. It is dead code, so I removed it.
</p>
</blockquote>
<p>
Excellent.
</p>
<blockquote class="citation">
<p>
If the API changes (which seems unlikely to me), then the fix will be very easy too.
</p>
</blockquote>
<p>
I think kini explained this sufficiently in <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:41" title="Comment 41">comment:41</a>. I don't care which way it's done.
</p>
<blockquote class="citation">
<blockquote class="citation">
<ul><li>I wonder about the not setting of the spines outward when the axes shouldn't cross. Here is an example which serves the point:
</li></ul></blockquote>
<p>
I will have to see how to handle this. Messing around with the spines was one of the primary reasons why setting scale wasn't working - the "converting masked to int" error.
</p>
</blockquote>
<p>
I see. It would be good to have consistency, since we went to some trouble to make them not cross any more.
</p>
<blockquote class="citation">
<p>
I think it is up to the user to either change their range, or their base, or provide custom ticks.
</p>
</blockquote>
<p>
Ah! You would think so. But we actually raise an error in the current code in precisely this situation. Presumably the code would be easy to just reuse?
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
But even with all of these comments, and waiting for the post-poll patch, <strong>fantastic</strong> job on this. Someone had to come along to finally wrap this for us, it's been requested zillions of times, and this is very worth the effort, thank you so much.
</p>
</blockquote>
<p>
Thanks. I needed it for my own research! :)
</p>
</blockquote>
<p>
Always good to have a good motivator! I've found that using things in class or for some random voting theory thing has ... enhanced my motivation to work on a topic.
</p>
<p>
Looking forward to seeing the global functions.
</p>
TicketppurkaSun, 27 May 2012 04:26:55 GMT
https://trac.sagemath.org/ticket/4529#comment:43
https://trac.sagemath.org/ticket/4529#comment:43
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:41" title="Comment 41">kini</a>:
</p>
<blockquote class="citation">
<p>
Oh, I didn't mean to prevent <code>\\mathdefault</code> from coming into the string at all. I meant to just specifically remove the substring <code>\\mathdefault</code> (say with <code>.replace("\\mathdefault","")</code> or something).
</p>
</blockquote>
<p>
indeed, the replace seems much better. Thanks.
</p>
TicketppurkaSun, 27 May 2012 09:12:18 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-check_for_single_tick.patch</em>
</li>
</ul>
<p>
apply to devel/sage
</p>
TicketppurkaSun, 27 May 2012 09:26:22 GMTdescription changed
https://trac.sagemath.org/ticket/4529#comment:44
https://trac.sagemath.org/ticket/4529#comment:44
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=44">diff</a>)
</li>
</ul>
<p>
The following are the changes in the latest patches:
</p>
<ol><li>labeling of minor ticks was flaky; it is fixed now.
</li><li>added several examples to <code>show()</code>
</li><li>fixed a problem in axes position of the matplotlib() function when custom xmax, xmin, etc were passed. Try this plot (without these patches) and then try with these patches :)
<pre class="wiki">plot(x).show(xmin=1, xmax=-1)
</pre></li><li>there is an <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-check_for_single_tick.patch" title="Attachment 'trac_4529-check_for_single_tick.patch' in Ticket #4529">optional patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-check_for_single_tick.patch" title="Download"></a> where we check for a single tick. this is tricky and I don't know a good and complete solution. Moreover, it makes most of the functions like 'arc, disk, etc' stop "just working" with log scale. It will be good if you have a better idea how to check for ticks which don't affect these functions too. We can't use xmin, xmax, etc to determine ticks because some of them might well be negative and matplotlib will neglect these values.
</li></ol>
TicketppurkaSun, 27 May 2012 09:39:26 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-add_logscale_to_Graphics.2.patch</em>
</li>
</ul>
<p>
apply to devel/sage
</p>
TicketkcrismanMon, 28 May 2012 05:40:57 GMT
https://trac.sagemath.org/ticket/4529#comment:45
https://trac.sagemath.org/ticket/4529#comment:45
<p>
Sorry for the delay - I only scheduled a couple hours of work this evening. Tomorrow I will have more time.
</p>
<p>
Hey, I just had an idea. Maybe this ticket should be <em>only</em> about the ticket description, and then another ticket for adding the "shortcut" commands like <code>plot_loglog</code> or whatever. In which case the current patches might be enough! (After much testing, of course.) What do you think?
</p>
<p>
More comments:
</p>
<ul><li>Wow, great catch on all those things like the minor ticks etc. I haven't really tested many of these plots yet, focusing on the code right now, so I'm glad you found a lot of those things.
</li><li>We'll eventually want to add some examples (or replace some of these) with the <code>plot(...,scale='loglog')</code> format ones. And definitely to add some to the file <code>plot.py</code>, since that is where a lot of people will look first for how to get this. <strong>But</strong> that can wait until the end, I'll be happy to do that in a reviewer patch.
</li><li>Catching this thing about the flipped <code>xmin/xmax</code> and friends is just a bonus - again, great catch and solution.
</li><li>As to the optional patch, I will think about this some more tomorrow. I do note that things still "work". For the example you removed <code>G</code>, the example you removed did in fact work, for instance.
<pre class="wiki">sage: G.show(scale=('loglog', 5)) # plots
sage: G.show(scale=('loglog', 4)) # plots
sage: G.show(scale=('loglog', 6)) # error
</pre>because it really depends on the <em>minor</em> ticks as well. I'm not sure whether having minor ticks should count as "having two ticks", especially in the relatively obscure-looking log plot situation. I'm not even sure why
<pre class="wiki">sage: G.show(scale=('loglog', 2))
</pre>works, since there is only one tick on the left!
</li><li>So all that said, I really am not sure why this is a problem. If someone is dumb enough to plot a CIRCLE with log scales, then they had better know how to pick the right scale so that this works. But the same would be true for plotting data, as you point out.
</li><li>Along those lines, your example (changed)
<pre class="wiki">sage: p = list_plot(range(1, 10), plotjoined=True)
sage: p.show(scale='loglog',base=2)
</pre>doesn't show any minor ticks. Is that just how <code>LogLocator</code> works, or is there something wrong?
</li></ul><p>
Obviously at this point we are getting close to nitpicking. I'll try more tomorrow.
</p>
TicketppurkaMon, 28 May 2012 06:04:56 GMT
https://trac.sagemath.org/ticket/4529#comment:46
https://trac.sagemath.org/ticket/4529#comment:46
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:45" title="Comment 45">kcrisman</a>:
</p>
<blockquote class="citation">
<p>
Sorry for the delay - I only scheduled a couple hours of work this evening. Tomorrow I will have more time.
</p>
</blockquote>
<p>
No problems. It is better to have a good implementation than to hurry through a bad one.
</p>
<blockquote class="citation">
<p>
Hey, I just had an idea. Maybe this ticket should be <em>only</em> about the ticket description, and then another ticket for adding the "shortcut" commands like <code>plot_loglog</code> or whatever. In which case the current patches might be enough! (After much testing, of course.) What do you think?
</p>
</blockquote>
<p>
Actually, there is zero change to be done to the code in <code>plot</code> or <code>list_plot</code>, except to add some examples. The only change that the additional patch will do is make very thin (one-liners) wrappers which defines the <code>plot_*</code> and <code>list_plot_*</code> wrappers and pass the correct scale option to <code>plot</code> and <code>list_plot</code>. The current consensus from the poll seems to favor this syntax. In view of this, the patch <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-add_docs_eg_to_some_user_facing_functions.patch" title="Attachment 'trac_4529-add_docs_eg_to_some_user_facing_functions.patch' in Ticket #4529">trac_4529-add_docs_eg_to_some_user_facing_functions.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-add_docs_eg_to_some_user_facing_functions.patch" title="Download"></a> requires only a renaming of the functions, and some extra examples.
</p>
<blockquote class="citation">
<ul><li>We'll eventually want to add some examples (or replace some of these) with the <code>plot(...,scale='loglog')</code> format ones. And definitely to add some to the file <code>plot.py</code>, since that is where a lot of people will look first for how to get this. <strong>But</strong> that can wait until the end, I'll be happy to do that in a reviewer patch.
</li></ul></blockquote>
<p>
We can add exactly similar examples to <code>plot()</code> and <code>list_plot()</code>. Currently, since the function is <code>Graphics.show()</code>, I decided to write it as <code>show(scale=...)</code> instead of <code>plot(scale=...)</code> since the former is more pertinent.
</p>
<blockquote class="citation">
<ul><li>Catching this thing about the flipped <code>xmin/xmax</code> and friends is just a bonus - again, great catch and solution.
</li><li>As to the optional patch, I will think about this some more tomorrow. I do note that things still "work". For the example you removed <code>G</code>, the example you removed did in fact work, for instance.
</li></ul></blockquote>
<p>
Yes I am aware that the examples I provided work. But most of the examples in the docs of those functions (arc, disk, contour_plot, etc) <em>do not</em> work out of the box, and we get the "too few ticks" error.
</p>
<blockquote class="citation">
<p>
I'm not sure whether having minor ticks should count as "having two ticks", especially in the relatively obscure-looking log plot situation. I'm not even sure why
</p>
<pre class="wiki">sage: G.show(scale=('loglog', 2))
</pre><blockquote>
<p>
works, since there is only one tick on the left!
</p>
</blockquote>
</blockquote>
<p>
Yes. That's why I said that the tick checking function is not robust and I also don't know how to make it robust.
</p>
<blockquote class="citation">
<ul><li>Along those lines, your example (changed)
<pre class="wiki">sage: p = list_plot(range(1, 10), plotjoined=True)
sage: p.show(scale='loglog',base=2)
</pre>doesn't show any minor ticks. Is that just how <code>LogLocator</code> works, or is there something wrong?
</li></ul></blockquote>
<p>
The minor ticks are generated by passing in multiples of 1/base to the subs parameter of <a class="missing wiki">LogLocator?</a>. So, for base 10, you will have ticks at say, <code>0.1*10^-1, 0.2*10^-1, ..., 0.9*10^-1, 10^-1</code>. This is a typical way how log plots are drawn in all programs (<code>pyplot.loglog()</code>, matlab, mathematica, etc). For <code>base=2</code> you can have only one tick <code>1/2*2^i</code> but this is a major tick at <code>2^(i-1)</code>. So, you will always see only one major tick and no minor ticks.
</p>
<blockquote class="citation">
<p>
Obviously at this point we are getting close to nitpicking. I'll try more tomorrow.
</p>
</blockquote>
<p>
Thanks a lot for the feedback. I am sure I have missed something, so I look forward to it. :)
</p>
TicketkcrismanMon, 28 May 2012 19:03:33 GMT
https://trac.sagemath.org/ticket/4529#comment:47
https://trac.sagemath.org/ticket/4529#comment:47
<blockquote class="citation">
<p>
Actually, there is zero change to be done to the code in <code>plot</code> or <code>list_plot</code>, except to add some examples. The only change that the additional patch will do is make very thin (one-liners) wrappers which defines the <code>plot_*</code> and <code>list_plot_*</code> wrappers and pass the correct scale option to <code>plot</code> and <code>list_plot</code>. The current consensus from the poll seems to favor this syntax. In view of this, the patch <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-add_docs_eg_to_some_user_facing_functions.patch" title="Attachment 'trac_4529-add_docs_eg_to_some_user_facing_functions.patch' in Ticket #4529">trac_4529-add_docs_eg_to_some_user_facing_functions.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-add_docs_eg_to_some_user_facing_functions.patch" title="Download"></a> requires only a renaming of the functions, and some extra examples.
</p>
</blockquote>
<p>
Oh, I didn't realize that that patch actually already implemented that - I hadn't had time to look at the patches until you had already said which patches to apply! Yes, I should be able to rebase/rename that well for our purposes, just adding a few more examples which occur fairly frequently.
</p>
<blockquote class="citation">
<p>
We can add exactly similar examples to <code>plot()</code> and <code>list_plot()</code>. Currently, since the function is <code>Graphics.show()</code>, I decided to write it as <code>show(scale=...)</code> instead of <code>plot(scale=...)</code> since the former is more pertinent.
</p>
</blockquote>
<p>
Of course.
</p>
<blockquote class="citation">
<blockquote class="citation">
<ul><li>As to the optional patch, I will think about this some more tomorrow. I do note that things still "work". For the example you removed <code>G</code>, the example you removed did in fact work, for instance.
</li></ul></blockquote>
<p>
Yes I am aware that the examples I provided work. But most of the examples in the docs of those functions (arc, disk, contour_plot, etc) <em>do not</em> work out of the box, and we get the "too few ticks" error.
</p>
</blockquote>
<p>
But they 'work', just not with the log scale, right?
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
I'm not sure whether having minor ticks should count as "having two ticks", especially in the relatively obscure-looking log plot situation. I'm not even sure why
</p>
<pre class="wiki">sage: G.show(scale=('loglog', 2))
</pre><blockquote>
<p>
works, since there is only one tick on the left!
</p>
</blockquote>
</blockquote>
<p>
Yes. That's why I said that the tick checking function is not robust and I also don't know how to make it robust.
</p>
</blockquote>
<p>
Haha. Well, I spent about a half-hour looking in depth at the code for <code>matplotlib.ticker.LogLocator</code> and have decided it is nearly impossible. There will very often, especially for <code>base=2</code> and friends, be ticks in the locator which are outside of the viewing range.
</p>
<p>
So there are ways to get some of this information... yuck. I may have something for this later today. I've been looking at it for over an hour and, at least for plots with only positive data, I think I have something. For ones with negative data it would be much more ugly and maybe not worth it.
</p>
<p>
By the way, I get
</p>
<pre class="wiki">sage: sage: disk((0,0), 1, (0, 3*pi/2)).show(scale='semilogx',base=2)
/Users/.../sage-5.1.beta0/local/lib/python2.7/site-packages/matplotlib/axes.py:1114: UserWarning: aspect is not supported for Axes with xscale=log, yscale=linear
% (xscale, yscale))
</pre><p>
when I try something that has an aspect ratio defined (disk, etc.)
</p>
<blockquote class="citation">
<p>
The minor ticks are generated by passing in multiples of 1/base to the subs parameter of <a class="missing wiki">LogLocator?</a>. So, for base 10, you will have ticks at say, <code>0.1*10^-1, 0.2*10^-1, ..., 0.9*10^-1, 10^-1</code>. This is a typical way how log plots are drawn in all programs (<code>pyplot.loglog()</code>, matlab, mathematica, etc). For <code>base=2</code> you can have only one tick <code>1/2*2^i</code> but this is a major tick at <code>2^(i-1)</code>. So, you will always see only one major tick and no minor ticks.
</p>
</blockquote>
<p>
I understood that, but didn't think through the implications for <code>base=2</code>. By the way, maybe changing
</p>
<pre class="wiki">srange(base_inv, 1+base_inv, base_inv)
</pre><p>
to
</p>
<pre class="wiki">srange(2*base_inv, 1, base_inv)
</pre><p>
would be useful, so that minor and major ticks don't overlap... probably doesn't matter, but could be ok.
</p>
TicketkcrismanMon, 28 May 2012 21:20:28 GMT
https://trac.sagemath.org/ticket/4529#comment:48
https://trac.sagemath.org/ticket/4529#comment:48
<p>
So I've come up with a solution. I'll probably post stuff in a little while. Here's a typical example.
</p>
<pre class="wiki">sage: P = plot(x^2,(x,1,8),scale='loglog'); P
ValueError: Either expand the range of the independent variable to allow two different integer powers of your `base`, or change your `base` to a smaller number.
sage: P.show(xmax=10)
</pre><p>
Some more food for thought.
</p>
<ul><li>Your LaTeX hack doesn't work in all cases, apparently.
<pre class="wiki">sage: P = plot(x^2,(x,1,8),scale='loglog', base=1.5); P
</pre>This is because matplotlib has
<pre class="wiki"> else:
if usetex:
s = r'$%s%d^{%d}$'% (sign_string, b, nearest_long(fx))
else:
s = r'$\mathdefault{%s%d^{%d}}$'% (sign_string, b,
nearest_long(fx))
</pre>which uses decimal formatting, but of course that only makes sense for integer bases. I feel like this is a bug in mpl; what do you think? Anyway, I am hesitant to patch our matplotlib just for this use case, though it should be a new ticket, I think. The same thing happens with <code>base=3/2</code>, unsurprisingly.
</li><li>This also bites your example with <code>base=float(e)</code> - did you look at the picture? Plain old <code>base=e</code> should be possible, but hacking around the fact that matplotlib won't accept this maybe makes it not worth it, and in any case I don't know that "real-life" people often use that for semilog plots. Just pointing it out.
</li><li>Here is a fun horrible example that you probably were thinking of with adding plots - maybe we should find a check for this.
<pre class="wiki">sage: G = plot_vector_field((e^x,e^(x+y)),(x,0.1,10),(y,0.1,10),scale=('loglog', 2))
sage: H = plot(x^2,(x,0,10),scale='linear')
sage: G+H
</pre>At least originally you thought this should raise an error.
</li></ul>
TicketkcrismanMon, 28 May 2012 23:28:38 GMT
https://trac.sagemath.org/ticket/4529#comment:49
https://trac.sagemath.org/ticket/4529#comment:49
<p>
Almost done with my changes! Here are things that definitely have to be decided or fixed.
</p>
<ul><li>What to do with noninteger bases. Tell people not to use them? Throw an exception? Hack mpl? Give copious examples of them looking bad?
</li><li>Decide what to do with adding plots.
</li></ul><p>
Other points.
</p>
<ul><li>I can't get <code>list_plot</code> to work with <code>scale='loglog'</code> or <code>'semilogx'</code> when passing it a list. From your examples -
<pre class="wiki"> sage: yl = [2**k for k in range(10)]
sage: list_plot(yl, scale='semilogy') # fine
sage: list_plot(yl, scale='loglog') # horiz. axis weird, no points
sage: list_plot_loglog(yl, base=2) # same weird
</pre>I assume this is because there is automatically a change to the list of tuples zipped with <code>range(n)</code>, so that there is always a zero involved in the horizontal axis. Here is the real problem.
<pre class="wiki">point([(0,1),(1,2),(2,3),(3,4),(4,5)],scale='semilogx',base=2) # doesn't work
</pre>What do you think the "correct" behavior is here?
</li></ul><p>
I fixed a bunch of minor doc issues. I also added a lot of
</p>
<pre class="wiki">
::
</pre><p>
between doctests which were meant to be viewed, because otherwise one will not be able to evaluate these plots in the live documentation (this is standard).
</p>
TicketkcrismanTue, 29 May 2012 00:05:05 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-add-log-scale.patch</em>
</li>
</ul>
TicketkcrismanTue, 29 May 2012 00:05:17 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-single-tick.patch</em>
</li>
</ul>
TicketkcrismanTue, 29 May 2012 00:05:29 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-docs-and-funcs.patch</em>
</li>
</ul>
TicketkcrismanTue, 29 May 2012 00:06:15 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-more-doc.patch</em>
</li>
</ul>
TicketkcrismanTue, 29 May 2012 00:09:22 GMTdescription, author changed
https://trac.sagemath.org/ticket/4529#comment:50
https://trac.sagemath.org/ticket/4529#comment:50
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=50">diff</a>)
</li>
<li><strong>author</strong>
changed from <em>Punarbasu Purkayastha</em> to <em>Punarbasu Purkayastha, Karl-Dieter Crisman</em>
</li>
</ul>
<p>
Okay, ppurka, ready for your comments on this whole mess. I've uploaded refreshed patches of yours, as well as a new one of doc fixes. The three issues (non-integer base, adding, list plots with list input) remain, so I am not putting "needs review" yet.
</p>
<p>
Patchbot: Apply trac_4529-add-log-scale.patch, trac_4529-single-tick.patch, trac_4529-docs-and-funcs.patch, and trac_4529-more-doc.patch.
</p>
TicketppurkaTue, 29 May 2012 03:10:37 GMT
https://trac.sagemath.org/ticket/4529#comment:51
https://trac.sagemath.org/ticket/4529#comment:51
<ul><li>trac_4529-add-log-scale.patch looks good.
</li><li>trac_4529-single-tick.patch also seems ok. I guess, we just have to leave it up to the common sense of the user in certain cases.
</li><li>trac_4529-docs-and-funcs.patch - I see that you fixed some of the typos!
</li><li>trac_4529-more-doc.patch - yes, this is good.
</li></ul><hr />
<ul><li>Adding another patch which fixes some more typos.
</li><li><code>list_plot</code> does seem to behave a bit weird sometimes. But if we use <code>plotjoined=True</code>, then it works fine. Thanks for figuring out where it goes awry. I will have a look into it. Should the fix go into a separate ticket?
</li><li>The plot commands in the documentation of <code>arc</code> and other functions don't usually work with the check for ticks and with log scale. This is even with <code>base=2</code>. For instance, the plot below. I guess we have to leave this up to the user to give a proper input.
<pre class="wiki">arc((2,3), 2, 1, angle=pi/5, sector=(0,pi/2)).show(scale='loglog', base=2)
arc((2,3), 2, 1, sector=(0,pi/2)).show(scale='loglog', base=2)
</pre></li><li>For some commands like <code>disk</code> where an aspect ratio warning is given by matplotlib, we can leave it up to the user I think. The fix is relatively simple though; see the code for <code>parametric_plot</code> in the <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-docs-and-funcs.patch" title="Attachment 'trac_4529-docs-and-funcs.patch' in Ticket #4529">trac_4529-docs-and-funcs.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-docs-and-funcs.patch" title="Download"></a>. If we fix <code>disk</code>, then we need to weed out all the cases for the rest of the commands in <code>sage.plot.*</code>.
</li><li>For adding plots - again we leave it up to the user. At present, adding log + linear works and matplotlib handles it. So, we should leave it at that. In my earlier code, I raised error because it was getting ambiguous what scale to set the <code>Graphics._xscale</code>, etc attribute to in case the user set the axis to log scale. It seems matplotlib prefers log over linear and sets everything to log.
</li></ul>
TicketppurkaTue, 29 May 2012 03:17:59 GMTstatus, reviewer, description changed
https://trac.sagemath.org/ticket/4529#comment:52
https://trac.sagemath.org/ticket/4529#comment:52
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>reviewer</strong>
changed from <em>Karl-Dieter Crisman</em> to <em>Karl-Dieter Crisman, Punarbasu Purkayastha</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=52">diff</a>)
</li>
</ul>
<p>
Added the typo fixes. I am good with your changes. So, it requires your review (as if you haven't reviewed enough already!) ;)
</p>
TicketppurkaTue, 29 May 2012 03:20:55 GMT
https://trac.sagemath.org/ticket/4529#comment:53
https://trac.sagemath.org/ticket/4529#comment:53
<p>
Forgot about noninteger bases: I think we should discourage the users from using them.
</p>
TicketppurkaTue, 29 May 2012 03:23:12 GMTstatus, work_issues changed
https://trac.sagemath.org/ticket/4529#comment:54
https://trac.sagemath.org/ticket/4529#comment:54
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
changed from <em>convenience functions</em> to <em>add warnings to list_plot and non-integer bases</em>
</li>
</ul>
TicketkcrismanTue, 29 May 2012 03:30:04 GMT
https://trac.sagemath.org/ticket/4529#comment:55
https://trac.sagemath.org/ticket/4529#comment:55
<p>
Thanks for the latest work!
</p>
<p>
In your latest patch, I see things like
</p>
<pre class="wiki">The `"semilogx"` scale
</pre><p>
But this will typeset as a LaTeX style thing, which is wrong. Try
</p>
<pre class="wiki">The ``"semilogx"`` scale
</pre><p>
and then all should be well with those. Easy enough to update on the last patch.
</p>
<blockquote class="citation">
<p>
Forgot about noninteger bases: I think we should discourage the users from using them.
</p>
</blockquote>
<p>
Hmm, ok. In that case maybe you can update your latest patch to still do the "e" example but make it really clear that this is (currently) not intended input? I don't want to actually check for integers, though; presumably in some contexts one could want 1.5. Do you think I should report this upstream to mpl?
</p>
<blockquote class="citation">
<p>
list_plot does seem to behave a bit weird sometimes. But if we use plotjoined=True, then it works fine. Thanks for figuring out where it goes awry. I will have a look into it. Should the fix go into a separate ticket?
</p>
</blockquote>
<p>
As long as we document that things can screw up on the x-axis for list_plot if you don't use plotjoined=True. That can go in your patch update too :)
</p>
<blockquote class="citation">
<p>
If we fix disk, then we need to weed out all the cases for the rest of the commands in sage.plot.*
</p>
</blockquote>
<p>
True. And I noticed the parametric business later on, good catch. I think this might be worth doing. Again, I'm not really worried - we had some fun laughing at the plots one gets from loglog plots of arcs and circles here. But anyway.
</p>
<blockquote class="citation">
<p>
It seems matplotlib prefers log over linear and sets everything to log.
</p>
</blockquote>
<p>
Oh, that explains what I saw. I have to say it was very weird.
</p>
<p>
I await the latest version of the last (?) patch.
</p>
TicketppurkaTue, 29 May 2012 04:27:42 GMT
https://trac.sagemath.org/ticket/4529#comment:56
https://trac.sagemath.org/ticket/4529#comment:56
<p>
Yes. Working on a lost patch. It should be up in a short while.
</p>
TicketppurkaTue, 29 May 2012 05:13:46 GMTstatus changed; work_issues deleted
https://trac.sagemath.org/ticket/4529#comment:57
https://trac.sagemath.org/ticket/4529#comment:57
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>work_issues</strong>
<em>add warnings to list_plot and non-integer bases</em> deleted
</li>
</ul>
<p>
I found my "lost" patch. *phew*
</p>
TicketppurkaTue, 29 May 2012 05:20:31 GMT
https://trac.sagemath.org/ticket/4529#comment:58
https://trac.sagemath.org/ticket/4529#comment:58
<p>
Reporting it to mpl is probably the right way to handle noninteger bases. They can check on their end, and change the labeling to <code>%.2f^{%d}</code> if a noninteger base is detected.
</p>
TicketkcrismanTue, 29 May 2012 06:43:38 GMT
https://trac.sagemath.org/ticket/4529#comment:59
https://trac.sagemath.org/ticket/4529#comment:59
<blockquote class="citation">
<p>
I found my "lost" patch. *phew*
</p>
</blockquote>
<p>
Good thing!
</p>
<p>
I've opened <a class="ext-link" href="https://github.com/matplotlib/matplotlib/issues/909"><span class="icon"></span>https://github.com/matplotlib/matplotlib/issues/909</a> for the mpl issue. Hopefully we'll hear something eventually.
</p>
<p>
Very minor request - on an update of your patch, perhaps.
</p>
<pre class="wiki">Note:
- Although it is possible to provide a noninteger ``base``, the
</pre><p>
maybe that could be in a standard
</p>
<pre class="wiki">.. note
Although it is possible ...
</pre><p>
which would render nicely in the built documentation.
</p>
<p>
The
</p>
<pre class="wiki">.. warning::
</pre><p>
should have a blank line after it.
</p>
<p>
Here's the complete list from <code>sage -docbuild reference html</code>.
</p>
<pre class="wiki">/Users/.../sage-5.1.beta0/local/lib/python2.7/site-packages/sage/plot/graphics.py:docstring of sage.plot.graphics.Graphics.axes_color:3: WARNING: Bullet list ends without a blank line; unexpected unindent.
/Users/.../sage-5.1.beta0/local/lib/python2.7/site-packages/sage/plot/graphics.py:docstring of sage.plot.graphics.Graphics.axes_color:5: ERROR: Unexpected indentation.
/Users/.../sage-5.1.beta0/local/lib/python2.7/site-packages/sage/plot/plot.py:docstring of sage.plot.plot:89: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/Users/.../sage-5.1.beta0/local/lib/python2.7/site-packages/sage/plot/plot.py:docstring of sage.plot.plot:36: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/Users/.../sage-5.1.beta0/local/lib/python2.7/site-packages/sage/plot/plot.py:docstring of sage.plot.plot:65: WARNING: Bullet list ends without a blank line; unexpected unindent.
/Users/.../sage-5.1.beta0/local/lib/python2.7/site-packages/sage/plot/plot.py:docstring of sage.plot.plot:67: ERROR: Unexpected indentation.
</pre><p>
Unfortunately it's too late now for me to properly test everything again - time for bed - but I am not really too concerned, given the stuff I've been trying all along. If you are able to deal with these formatting issues, I'd be grateful, and even if I have to deal with it slowly we should certainly be ready for positive review by the end of the week. Hopefully by tomorrow!
</p>
TicketppurkaTue, 29 May 2012 07:17:00 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-typo_fixes.patch</em>
</li>
</ul>
TicketppurkaTue, 29 May 2012 07:18:41 GMT
https://trac.sagemath.org/ticket/4529#comment:60
https://trac.sagemath.org/ticket/4529#comment:60
<p>
updated the last patch. It should fix the rest of the warnings.
</p>
<pre class="wiki">.../sage-5.1beta0/devel/sage» ../../sage -b && ../../sage -docbuild reference html
----------------------------------------------------------
sage: Building and installing modified Sage library files.
Installing c_lib
scons: `install' is up to date.
Updating Cython code....
setup.py:650: UserWarning: could not find dependency <vector> included in /home/punarbasu/Installations/sage-5.1beta0/local/lib/python/site-packages/Cython/Includes/libcpp/vector.pxd. I will assume it is a system C/C++ header.
warnings.warn(msg+' I will assume it is a system C/C++ header.')
setup.py:650: UserWarning: could not find dependency <string> included in /home/punarbasu/Installations/sage-5.1beta0/local/lib/python/site-packages/Cython/Includes/libcpp/string.pxd. I will assume it is a system C/C++ header.
warnings.warn(msg+' I will assume it is a system C/C++ header.')
Executing 0 commands (using 0 threads)
Time to execute 0 commands: 0.0459749698639 seconds
Finished compiling Cython code (time = 0.499665021896 seconds)
running install
running build
running build_py
copying sage/plot/plot.py -> build/lib.linux-x86_64-2.7/sage/plot
running build_ext
warning: Replacing library search directory in linker command:
"/home/punarbasu/Installations/sage-5.0.rc0/local/lib" -> "/home/punarbasu/Installations/sage-5.1beta0/local/lib"
Executing 0 commands (using 0 threads)
Time to execute 0 commands: 0.00115895271301 seconds
Total time spent compiling C/C++ extensions: 0.0369729995728 seconds.
running install_lib
copying build/lib.linux-x86_64-2.7/sage/plot/plot.py -> /home/punarbasu/Installations/sage-5.1beta0/local/lib/python2.7/site-packages/sage/plot
byte-compiling /home/punarbasu/Installations/sage-5.1beta0/local/lib/python2.7/site-packages/sage/plot/plot.py to plot.pyc
running install_egg_info
Removing /home/punarbasu/Installations/sage-5.1beta0/local/lib/python2.7/site-packages/sage-0.0.0-py2.7.egg-info
Writing /home/punarbasu/Installations/sage-5.1beta0/local/lib/python2.7/site-packages/sage-0.0.0-py2.7.egg-info
real 0m1.513s
user 0m1.202s
sys 0m0.206s
sphinx-build -b html -d /home/punarbasu/Installations/sage-5.1beta0/devel/sage/doc/output/doctrees/en/reference /home/punarbasu/Installations/sage-5.1beta0/devel/sage/doc/en/reference /home/punarbasu/Installations/sage-5.1beta0/devel/sage/doc/output/html/en/reference
Running Sphinx v1.1.2
loading pickled environment... done
loading intersphinx inventory from /home/punarbasu/Installations/sage-5.1beta0/devel/sage/doc/common/python.inv...
building [html]: targets for 1 source files that are out of date
updating environment: 0 added, 1 changed, 0 removed
reading sources... [100%] sage/plot/plot
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
WARNING: dvipng command 'dvipng' cannot be run (needed for math display), check the pngmath_dvipng setting
writing output... [100%] sage/plot/plot
writing additional files... genindex py-modindex search
copying static files... done
dumping search index... done
dumping object inventory... done
build succeeded, 1 warning.
Build finished. The built documents can be found in /home/punarbasu/Installations/sage-5.1beta0/devel/sage/doc/output/html/en/reference
</pre>
TicketkcrismanTue, 29 May 2012 17:27:39 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-final-fixes.patch</em>
</li>
</ul>
TicketkcrismanTue, 29 May 2012 17:28:45 GMTdescription changed
https://trac.sagemath.org/ticket/4529#comment:61
https://trac.sagemath.org/ticket/4529#comment:61
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=61">diff</a>)
</li>
</ul>
<p>
Patchbot:
</p>
<p>
Apply
</p>
<ul><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-add-log-scale.patch" title="Attachment 'trac_4529-add-log-scale.patch' in Ticket #4529">trac_4529-add-log-scale.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-add-log-scale.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-single-tick.patch" title="Attachment 'trac_4529-single-tick.patch' in Ticket #4529">trac_4529-single-tick.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-single-tick.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-docs-and-funcs.patch" title="Attachment 'trac_4529-docs-and-funcs.patch' in Ticket #4529">trac_4529-docs-and-funcs.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-docs-and-funcs.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-more-doc.patch" title="Attachment 'trac_4529-more-doc.patch' in Ticket #4529">trac_4529-more-doc.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-more-doc.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-typo_fixes.patch" title="Attachment 'trac_4529-typo_fixes.patch' in Ticket #4529">trac_4529-typo_fixes.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-typo_fixes.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-final-fixes.patch" title="Attachment 'trac_4529-final-fixes.patch' in Ticket #4529">trac_4529-final-fixes.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-final-fixes.patch" title="Download"></a>
</li></ul>
TicketkcrismanTue, 29 May 2012 17:32:26 GMT
https://trac.sagemath.org/ticket/4529#comment:62
https://trac.sagemath.org/ticket/4529#comment:62
<p>
Okay, this morning I had time to make one final formatting change and add a few more tests, and change some that would not have looked right. In particular I put the whole warning thing into one warning block - cosmetic, but useful.
</p>
<p>
I think there are probably a few more corner case things we haven't thought of, but the functionality is now robust enough and well-documented enough in the corner cases we did discover that it's ready for positive review. Although some disagree with this philosophy, my opinion is that for a project like Sage it is better to add new functionality that is 99% ready and then provide quick fixes to minor issues that remain; we simply don't have the resources to spend days trying to break it. If you're happy with it, I'm happy with it. What do you think?
</p>
TicketppurkaWed, 30 May 2012 01:30:52 GMT
https://trac.sagemath.org/ticket/4529#comment:63
https://trac.sagemath.org/ticket/4529#comment:63
<p>
I am happy with the changes. :)
</p>
<p>
NB: The patchbot won't work, because it needs to apply the dependency ticket first.
</p>
TicketkiniWed, 30 May 2012 02:53:48 GMT
https://trac.sagemath.org/ticket/4529#comment:64
https://trac.sagemath.org/ticket/4529#comment:64
<p>
The patchbot will work - I will make it work!
</p>
<p>
Patchbot, listen carefully! apply trac_4529-add-log-scale.patch trac_4529-single-tick.patch trac_4529-docs-and-funcs.patch trac_4529-more-doc.patch trac_4529-typo_fixes.patch trac_4529-final-fixes.patch !!!
</p>
TicketjasonWed, 30 May 2012 03:31:13 GMT
https://trac.sagemath.org/ticket/4529#comment:65
https://trac.sagemath.org/ticket/4529#comment:65
<p>
I was hoping you'd say: "Patch bot, these are the files you are looking for" (I hope that space makes this comment really just a comment!) (and, uh, that was a star wars reference...)
</p>
TicketppurkaWed, 30 May 2012 03:51:41 GMT
https://trac.sagemath.org/ticket/4529#comment:66
https://trac.sagemath.org/ticket/4529#comment:66
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:65" title="Comment 65">jason</a>:
</p>
<blockquote class="citation">
<p>
I was hoping you'd say: "Patch bot, these are the files you are looking for" (I hope that space makes this comment really just a comment!) (and, uh, that was a star wars reference...)
</p>
</blockquote>
<p>
Ha ha! The bot listens only to kini!
</p>
TicketkiniWed, 30 May 2012 03:56:40 GMT
https://trac.sagemath.org/ticket/4529#comment:67
https://trac.sagemath.org/ticket/4529#comment:67
<p>
You don't actually have to address the patchbot to make it understand - you just need the word "apply" followed by the correct patch names in the correct order in plain text, possibly on the same line, I don't remember. I just mention the patchbot so people don't wonder what the cryptic comment with patch filenames in it is about.
</p>
TicketkcrismanWed, 30 May 2012 04:31:02 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-typo_fixes-rebase.patch</em>
</li>
</ul>
TicketkcrismanWed, 30 May 2012 04:34:17 GMTstatus, dependencies, description changed
https://trac.sagemath.org/ticket/4529#comment:68
https://trac.sagemath.org/ticket/4529#comment:68
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>dependencies</strong>
changed from <em>#12974</em> to <em>#12810, #12605, #12974</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=68">diff</a>)
</li>
</ul>
<p>
Luckily I only had to rebase one of these, due to some fuzz which the release manager doesn't always like. Otherwise this is good to go! Note the long list of dependencies, only so that all patches apply properly on 5.1.beta1.
</p>
<p>
Positive review!
</p>
<p>
Patchbot, apply apply trac_4529-add-log-scale.patch trac_4529-single-tick.patch trac_4529-docs-and-funcs.patch trac_4529-more-doc.patch trac_4529-typo_fixes-rebase.patch trac_4529-final-fixes.patch
</p>
TicketppurkaWed, 30 May 2012 06:53:54 GMTdescription changed
https://trac.sagemath.org/ticket/4529#comment:69
https://trac.sagemath.org/ticket/4529#comment:69
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=69">diff</a>)
</li>
</ul>
TicketppurkaWed, 30 May 2012 06:55:30 GMTdescription changed
https://trac.sagemath.org/ticket/4529#comment:70
https://trac.sagemath.org/ticket/4529#comment:70
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=70">diff</a>)
</li>
</ul>
TicketppurkaWed, 30 May 2012 06:57:06 GMTdescription changed
https://trac.sagemath.org/ticket/4529#comment:71
https://trac.sagemath.org/ticket/4529#comment:71
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=71">diff</a>)
</li>
</ul>
TicketppurkaWed, 30 May 2012 06:58:22 GMTdescription changed
https://trac.sagemath.org/ticket/4529#comment:72
https://trac.sagemath.org/ticket/4529#comment:72
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=72">diff</a>)
</li>
</ul>
TicketppurkaWed, 30 May 2012 10:56:03 GMTdescription changed
https://trac.sagemath.org/ticket/4529#comment:73
https://trac.sagemath.org/ticket/4529#comment:73
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=73">diff</a>)
</li>
</ul>
<p>
Patchbot apply: trac_4529-add-log-scale.patch trac_4529-single-tick.patch trac_4529-docs-and-funcs.patch trac_4529-more-doc.patch trac_4529-typo_fixes-rebase.patch trac_4529-final-fixes.patch
</p>
TicketppurkaSat, 02 Jun 2012 02:52:17 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-docs-and-funcs.2.patch</em>
</li>
</ul>
TicketppurkaSat, 02 Jun 2012 02:53:32 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-typo_fixes-rebase.2.patch</em>
</li>
</ul>
TicketppurkaSat, 02 Jun 2012 02:56:44 GMTdescription changed
https://trac.sagemath.org/ticket/4529#comment:74
https://trac.sagemath.org/ticket/4529#comment:74
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=74">diff</a>)
</li>
</ul>
<p>
I noticed that the case when <code>scale</code> is a list wasn't handled for changing aspect ratio in the new code. It's a non-fatal case, but it is nice to make sure that user doesn't get warnings from mpl. Updated two of the patches. Changes can be <a class="ext-link" href="https://github.com/ppurka/sage-patches/commit/36f9d7997ca3d6023c84d5146bc138a9952f91cc"><span class="icon"></span>seen here</a>.
</p>
<p>
Patchbot apply: trac_4529-add-log-scale.patch trac_4529-single-tick.patch trac_4529-docs-and-funcs.2.patch trac_4529-more-doc.patch trac_4529-typo_fixes-rebase.2.patch trac_4529-final-fixes.patch
</p>
TicketkcrismanSat, 02 Jun 2012 03:25:33 GMT
https://trac.sagemath.org/ticket/4529#comment:75
https://trac.sagemath.org/ticket/4529#comment:75
<p>
Okay by me as long as it all still applies, as it appears you have been very careful to ensure! You really do find some good corner cases!
</p>
TicketjdemeyerMon, 18 Jun 2012 13:35:34 GMTmilestone changed
https://trac.sagemath.org/ticket/4529#comment:76
https://trac.sagemath.org/ticket/4529#comment:76
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.1</em> to <em>sage-5.2</em>
</li>
</ul>
TicketjdemeyerThu, 28 Jun 2012 13:28:12 GMTmilestone changed
https://trac.sagemath.org/ticket/4529#comment:77
https://trac.sagemath.org/ticket/4529#comment:77
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.2</em> to <em>sage-pending</em>
</li>
</ul>
TicketkcrismanThu, 28 Jun 2012 13:37:34 GMT
https://trac.sagemath.org/ticket/4529#comment:78
https://trac.sagemath.org/ticket/4529#comment:78
<p>
I'll try to rebase <a class="closed ticket" href="https://trac.sagemath.org/ticket/12974" title="enhancement: make Graphics class inheritable and some clean ups (closed: fixed)">#12974</a> this morning.
</p>
TicketjdemeyerThu, 28 Jun 2012 13:43:53 GMT
https://trac.sagemath.org/ticket/4529#comment:79
https://trac.sagemath.org/ticket/4529#comment:79
<p>
Note that <a class="closed ticket" href="https://trac.sagemath.org/ticket/12974" title="enhancement: make Graphics class inheritable and some clean ups (closed: fixed)">#12974</a> needs work though...
</p>
TicketkcrismanThu, 28 Jun 2012 14:31:55 GMT
https://trac.sagemath.org/ticket/4529#comment:80
https://trac.sagemath.org/ticket/4529#comment:80
<blockquote class="citation">
<p>
Note that <a class="closed ticket" href="https://trac.sagemath.org/ticket/12974" title="enhancement: make Graphics class inheritable and some clean ups (closed: fixed)">#12974</a> needs work though...
</p>
</blockquote>
<p>
Yes, I saw that. It looks like they are all the result of just one name-mangling change or something, though, so hopefully not a problem.
</p>
TicketkcrismanThu, 28 Jun 2012 15:02:54 GMT
https://trac.sagemath.org/ticket/4529#comment:81
https://trac.sagemath.org/ticket/4529#comment:81
<p>
Okay, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12974" title="enhancement: make Graphics class inheritable and some clean ups (closed: fixed)">#12974</a> seems to be all set, so one would just have to check that this still applied on top of that.
</p>
TicketkcrismanThu, 28 Jun 2012 18:10:31 GMT
https://trac.sagemath.org/ticket/4529#comment:82
https://trac.sagemath.org/ticket/4529#comment:82
<p>
Yuck. Lots of places it doesn't apply correctly. I'll try to take care of this now.
</p>
TicketkcrismanThu, 28 Jun 2012 18:34:05 GMT
https://trac.sagemath.org/ticket/4529#comment:83
https://trac.sagemath.org/ticket/4529#comment:83
<p>
Every single conflict was due to whitespace (and hence probably fallout from <a class="closed ticket" href="https://trac.sagemath.org/ticket/12974" title="enhancement: make Graphics class inheritable and some clean ups (closed: fixed)">#12974</a>'s extra patch. Grr. Patches coming up.
</p>
TicketjasonThu, 28 Jun 2012 18:39:51 GMT
https://trac.sagemath.org/ticket/4529#comment:84
https://trac.sagemath.org/ticket/4529#comment:84
<p>
Wasn't the whitespace patch from <a class="closed ticket" href="https://trac.sagemath.org/ticket/12974" title="enhancement: make Graphics class inheritable and some clean ups (closed: fixed)">#12974</a> removed? Why are there still conflicts?
</p>
TicketkcrismanThu, 28 Jun 2012 18:43:16 GMT
https://trac.sagemath.org/ticket/4529#comment:85
https://trac.sagemath.org/ticket/4529#comment:85
<blockquote class="citation">
<p>
Wasn't the whitespace patch from <a class="closed ticket" href="https://trac.sagemath.org/ticket/12974" title="enhancement: make Graphics class inheritable and some clean ups (closed: fixed)">#12974</a> removed? Why are there still conflicts?
</p>
</blockquote>
<p>
It <em>was</em> removed. But these patches are all based on the 'old' <a class="closed ticket" href="https://trac.sagemath.org/ticket/12974" title="enhancement: make Graphics class inheritable and some clean ups (closed: fixed)">#12974</a>. There would <em>not</em> have been conflicts if we had kept that patch.
</p>
<p>
Anyway, I just finished, so hold on a sec. It would be good for someone to look at things to make sure I did the rebasing right.
</p>
TicketkcrismanThu, 28 Jun 2012 19:15:22 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-patch1.patch</em>
</li>
</ul>
TicketkcrismanThu, 28 Jun 2012 19:15:36 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-patch2.patch</em>
</li>
</ul>
TicketkcrismanThu, 28 Jun 2012 19:15:48 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-patch3.patch</em>
</li>
</ul>
TicketkcrismanThu, 28 Jun 2012 19:16:23 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-patch5.patch</em>
</li>
</ul>
TicketkcrismanThu, 28 Jun 2012 19:16:40 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-patch6.patch</em>
</li>
</ul>
TicketkcrismanThu, 28 Jun 2012 19:19:26 GMTdescription changed
https://trac.sagemath.org/ticket/4529#comment:86
https://trac.sagemath.org/ticket/4529#comment:86
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4529?action=diff&version=86">diff</a>)
</li>
</ul>
<p>
Sorry for all the patches, but I was trying to make sure things looked the same. These are the same six patches, just in numerical number - didn't want to add more rebase suffixes.
</p>
<p>
Apply
</p>
<ul><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-patch1.patch" title="Attachment 'trac_4529-patch1.patch' in Ticket #4529">trac_4529-patch1.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-patch1.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-patch2.patch" title="Attachment 'trac_4529-patch2.patch' in Ticket #4529">trac_4529-patch2.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-patch2.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-patch3.patch" title="Attachment 'trac_4529-patch3.patch' in Ticket #4529">trac_4529-patch3.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-patch3.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-patch4.patch" title="Attachment 'trac_4529-patch4.patch' in Ticket #4529">trac_4529-patch4.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-patch4.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-patch5.patch" title="Attachment 'trac_4529-patch5.patch' in Ticket #4529">trac_4529-patch5.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-patch5.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-patch6.patch" title="Attachment 'trac_4529-patch6.patch' in Ticket #4529">trac_4529-patch6.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-patch6.patch" title="Download"></a>
</li></ul>
TicketkcrismanThu, 28 Jun 2012 19:32:40 GMT
https://trac.sagemath.org/ticket/4529#comment:87
https://trac.sagemath.org/ticket/4529#comment:87
<p>
Plots seem fine still.
</p>
<p>
With respect to one minor issue - this is nothing to do with positive review, but a question for ppurka.
</p>
<p>
Compare
</p>
<pre class="wiki">plot(x,(x,-1,1),xmin=1,xmax=-1)
plot(x,(x,-1,1)).show(xmin=1,xmax=-1)
</pre><p>
with the patches (or even just first patch). I would think they should be the same (i.e., like the second one, as commented in the first patch).
</p>
TicketppurkaThu, 28 Jun 2012 19:37:29 GMT
https://trac.sagemath.org/ticket/4529#comment:88
https://trac.sagemath.org/ticket/4529#comment:88
<p>
Hello, sorry for not being able to participate. Currntly posting from an airport kiosk. I thought they should be the same. I will have to check if the code is only triggered from matplotlib, or if the points are sorted before the <code>render_to_subplot</code>.
</p>
TicketppurkaThu, 28 Jun 2012 19:41:58 GMT
https://trac.sagemath.org/ticket/4529#comment:89
https://trac.sagemath.org/ticket/4529#comment:89
<p>
By the way many thanks to you (kcrisman), jason and jeroen for doing the rebasing :)
</p>
TicketkcrismanThu, 28 Jun 2012 19:51:04 GMT
https://trac.sagemath.org/ticket/4529#comment:90
https://trac.sagemath.org/ticket/4529#comment:90
<blockquote class="citation">
<p>
Hello, sorry for not being able to participate. Currntly posting from an airport kiosk. I thought they should be the same. I will have to check if the code is only triggered from matplotlib, or if the points are sorted before the <code>render_to_subplot</code>.
</p>
</blockquote>
<p>
I wouldn't be surprised if they were.
</p>
<p>
Anyway, when you figure it out, open a ticket :) Have a great trip, wherever you go.
</p>
TicketjdemeyerFri, 29 Jun 2012 15:45:55 GMT
https://trac.sagemath.org/ticket/4529#comment:91
https://trac.sagemath.org/ticket/4529#comment:91
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:86" title="Comment 86">kcrisman</a>:
</p>
<blockquote class="citation">
<p>
Sorry for all the patches, but I was trying to make sure things looked the same. These are the same six patches, just in numerical number
</p>
</blockquote>
<p>
Awesome!
</p>
TicketjdemeyerMon, 02 Jul 2012 15:30:36 GMTmilestone changed
https://trac.sagemath.org/ticket/4529#comment:92
https://trac.sagemath.org/ticket/4529#comment:92
<ul>
<li><strong>milestone</strong>
changed from <em>sage-pending</em> to <em>sage-5.2</em>
</li>
</ul>
TicketjdemeyerTue, 03 Jul 2012 09:55:22 GMTstatus changed
https://trac.sagemath.org/ticket/4529#comment:93
https://trac.sagemath.org/ticket/4529#comment:93
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_info</em>
</li>
</ul>
<p>
<a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-patch4.patch" title="Attachment 'trac_4529-patch4.patch' in Ticket #4529">trac_4529-patch4.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-patch4.patch" title="Download"></a> needs a proper commit message.
</p>
TicketkcrismanTue, 03 Jul 2012 13:17:47 GMT
https://trac.sagemath.org/ticket/4529#comment:94
https://trac.sagemath.org/ticket/4529#comment:94
<blockquote class="citation">
<p>
<a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4529/trac_4529-patch4.patch" title="Attachment 'trac_4529-patch4.patch' in Ticket #4529">trac_4529-patch4.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4529/trac_4529-patch4.patch" title="Download"></a> needs a proper commit message.
</p>
</blockquote>
<p>
Huh, that happened the last time too. Sorry, I don't know how I missed that. Coming up.
</p>
TicketkcrismanTue, 03 Jul 2012 13:18:13 GMTattachment set
https://trac.sagemath.org/ticket/4529
https://trac.sagemath.org/ticket/4529
<ul>
<li><strong>attachment</strong>
set to <em>trac_4529-patch4.patch</em>
</li>
</ul>
TicketkcrismanTue, 03 Jul 2012 13:18:40 GMTstatus changed
https://trac.sagemath.org/ticket/4529#comment:95
https://trac.sagemath.org/ticket/4529#comment:95
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
</ul>
TicketkcrismanTue, 03 Jul 2012 13:18:56 GMTstatus changed
https://trac.sagemath.org/ticket/4529#comment:96
https://trac.sagemath.org/ticket/4529#comment:96
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Ok, should be all set.
</p>
TicketjdemeyerSat, 07 Jul 2012 22:28:35 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/4529#comment:97
https://trac.sagemath.org/ticket/4529#comment:97
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage-5.2.beta1</em>
</li>
</ul>
TicketkcrismanWed, 15 Aug 2012 14:00:41 GMT
https://trac.sagemath.org/ticket/4529#comment:98
https://trac.sagemath.org/ticket/4529#comment:98
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4529#comment:59" title="Comment 59">kcrisman</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
I found my "lost" patch. *phew*
</p>
</blockquote>
<p>
Good thing!
</p>
<p>
I've opened <a class="ext-link" href="https://github.com/matplotlib/matplotlib/issues/909"><span class="icon"></span>https://github.com/matplotlib/matplotlib/issues/909</a> for the mpl issue. Hopefully we'll hear something eventually.
</p>
</blockquote>
<p>
ppurka:
</p>
<p>
Did we ever open a ticket for fixing this? Anyway, <a class="ext-link" href="https://github.com/matplotlib/matplotlib/issues/909#issuecomment-7756212"><span class="icon"></span>it's been resolved</a> in <a class="ext-link" href="https://github.com/matplotlib/matplotlib/pull/960"><span class="icon"></span>this mpl pull request</a>, apparently. I <em>think</em> that we put in a comment about how this doesn't (yet) work, so there would be something to do now that this has been done.
</p>
Ticket