Sage: Ticket #8734: make sage variables unique in maxima
https://trac.sagemath.org/ticket/8734
<p>
This patch prepends a unique string, "_SAGE_VAR_", to each variable name in maxima, to avoid conflicts with existing variables in maxima.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/8734
Trac 1.2Jason GroutWed, 21 Apr 2010 05:52:47 GMTattachment set
https://trac.sagemath.org/ticket/8734
https://trac.sagemath.org/ticket/8734
<ul>
<li><strong>attachment</strong>
set to <em>trac-8734-maxima-vars.patch</em>
</li>
</ul>
TicketJason GroutWed, 21 Apr 2010 05:54:17 GMTstatus changed; cc set
https://trac.sagemath.org/ticket/8734#comment:1
https://trac.sagemath.org/ticket/8734#comment:1
<ul>
<li><strong>cc</strong>
<em>William Stein</em> <em>Alex Leone</em> added
</li>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_work</em>
</li>
</ul>
<p>
I haven't run doctests on everything, but this patch is a start, if one of you wants to take it from here.
</p>
TicketKarl-Dieter CrismanWed, 21 Apr 2010 13:03:51 GMTcc changed
https://trac.sagemath.org/ticket/8734#comment:2
https://trac.sagemath.org/ticket/8734#comment:2
<ul>
<li><strong>cc</strong>
<em>Karl-Dieter Crisman</em> added
</li>
</ul>
<p>
I think there might also be another ticket about this somewhere...
</p>
TicketRobert MarikWed, 21 Apr 2010 13:23:39 GMT
https://trac.sagemath.org/ticket/8734#comment:3
https://trac.sagemath.org/ticket/8734#comment:3
<p>
Hi, it is also <a class="closed ticket" href="https://trac.sagemath.org/ticket/8634" title="#8634: defect: Broken conversion of sage variable h1 to Maxima (closed: duplicate)">#8634</a> and there is a dicussion at <a class="ext-link" href="http://groups.google.com/group/sage-devel/browse_thread/thread/67f0a63d00b8d835/06557a921a582f87"><span class="icon"></span>sage-devel</a>
</p>
TicketRobert MarikWed, 21 Apr 2010 16:27:20 GMTcc changed
https://trac.sagemath.org/ticket/8734#comment:4
https://trac.sagemath.org/ticket/8734#comment:4
<ul>
<li><strong>cc</strong>
<em>Robert Marik</em> added
</li>
</ul>
TicketJason GroutMon, 03 May 2010 17:36:09 GMTowner changed
https://trac.sagemath.org/ticket/8734#comment:5
https://trac.sagemath.org/ticket/8734#comment:5
<ul>
<li><strong>owner</strong>
changed from <em>William Stein</em> to <em>Jason Grout</em>
</li>
</ul>
<p>
This should fix <a class="closed ticket" href="https://trac.sagemath.org/ticket/6882" title="#6882: defect: bugs in conversion of variable names from Maxima to Sage (closed: fixed)">#6882</a>
</p>
TicketJason GroutMon, 03 May 2010 17:37:15 GMT
https://trac.sagemath.org/ticket/8734#comment:6
https://trac.sagemath.org/ticket/8734#comment:6
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8734#comment:5" title="Comment 5">jason</a>:
</p>
<blockquote class="citation">
<p>
This should fix <a class="closed ticket" href="https://trac.sagemath.org/ticket/6882" title="#6882: defect: bugs in conversion of variable names from Maxima to Sage (closed: fixed)">#6882</a>
</p>
</blockquote>
<p>
Well, or at least *help* with the solution, anyway.
</p>
TicketBurcin ErocalTue, 04 May 2010 01:56:07 GMTcc changed
https://trac.sagemath.org/ticket/8734#comment:7
https://trac.sagemath.org/ticket/8734#comment:7
<ul>
<li><strong>cc</strong>
<em>Burcin Erocal</em> added
</li>
</ul>
TicketFranco SaliolaFri, 18 Jun 2010 13:11:59 GMTcc changed
https://trac.sagemath.org/ticket/8734#comment:8
https://trac.sagemath.org/ticket/8734#comment:8
<ul>
<li><strong>cc</strong>
<em>Franco Saliola</em> added
</li>
</ul>
TicketMitesh PatelThu, 12 Aug 2010 22:47:31 GMT
https://trac.sagemath.org/ticket/8734#comment:9
https://trac.sagemath.org/ticket/8734#comment:9
<p>
This ticket may also fix <a class="closed ticket" href="https://trac.sagemath.org/ticket/9538" title="#9538: defect: internal side effect in roots? (closed: fixed)">#9538</a>.
</p>
TicketJeroen DemeyerTue, 13 Aug 2013 15:35:53 GMTmilestone changed
https://trac.sagemath.org/ticket/8734#comment:10
https://trac.sagemath.org/ticket/8734#comment:10
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.11</em> to <em>sage-5.12</em>
</li>
</ul>
TicketPaul ZimmermannSun, 25 Aug 2013 13:09:45 GMTwork_issues set
https://trac.sagemath.org/ticket/8734#comment:11
https://trac.sagemath.org/ticket/8734#comment:11
<ul>
<li><strong>work_issues</strong>
set to <em>rebase</em>
</li>
</ul>
<p>
patch should be rebased, it fails to apply to Sage 5.11:
</p>
<pre class="wiki">sage: hg_sage.import_patch("/tmp/trac-8734-maxima-vars.patch")
cd "/home/zimmerma/Downloads/sage-5.11/devel/sage" && sage --hg import "/tmp/trac-8734-maxima-vars.patch"
applying /tmp/trac-8734-maxima-vars.patch
patching file sage/calculus/calculus.py
Hunk #1 FAILED at 1450
Hunk #2 FAILED at 1461
2 out of 2 hunks FAILED -- saving rejects to file sage/calculus/calculus.py.rej
patching file sage/symbolic/assumptions.py
Hunk #1 FAILED at 100
1 out of 1 hunks FAILED -- saving rejects to file sage/symbolic/assumptions.py.rej
abort: patch failed to apply
</pre><p>
Paul
</p>
TicketFor batch modificationsThu, 30 Jan 2014 21:20:52 GMTmilestone changed
https://trac.sagemath.org/ticket/8734#comment:12
https://trac.sagemath.org/ticket/8734#comment:12
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.1</em> to <em>sage-6.2</em>
</li>
</ul>
TicketRalf StephanSat, 08 Mar 2014 08:28:53 GMTchangetime changed; branch set
https://trac.sagemath.org/ticket/8734#comment:13
https://trac.sagemath.org/ticket/8734#comment:13
<ul>
<li><strong>changetime</strong>
changed from <em>Jan 30, 2014, 9:20:52 PM</em> to <em>Jan 30, 2014, 9:20:52 PM</em>
</li>
<li><strong>branch</strong>
set to <em>u/rws/ticket/8734</em>
</li>
</ul>
TicketRalf StephanSat, 08 Mar 2014 08:30:52 GMTreviewer, commit set
https://trac.sagemath.org/ticket/8734#comment:14
https://trac.sagemath.org/ticket/8734#comment:14
<ul>
<li><strong>reviewer</strong>
set to <em>Ralf Stephan</em>
</li>
<li><strong>commit</strong>
set to <em>87d944ffbe59afe29de2faea2b569f7eaa904295</em>
</li>
</ul>
<p>
Rebased on 6.2.beta3
</p>
<p>
Patch doesn't seem ready:
</p>
<pre class="wiki">sage -t --long src/sage/symbolic/integration/integral.py # 3 doctests failed
sage -t --long src/sage/symbolic/assumptions.py # 26 doctests failed
sage -t --long src/sage/symbolic/pynac.pyx # 1 doctest failed
sage -t --long src/sage/symbolic/expression.pyx # 21 doctests failed
sage -t --long src/sage/calculus/desolvers.py # 63 doctests failed
sage -t --long src/sage/calculus/calculus.py # 13 doctests failed
sage -t --long src/sage/calculus/functional.py # 1 doctest failed
</pre><hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=87d944ffbe59afe29de2faea2b569f7eaa904295"><span class="icon"></span>87d944f</a></td><td><code>Trac #8734: Make sage variables unique in maxima.</code>
</td></tr></table>
TicketgitSat, 22 Mar 2014 15:29:13 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:15
https://trac.sagemath.org/ticket/8734#comment:15
<ul>
<li><strong>commit</strong>
changed from <em>87d944ffbe59afe29de2faea2b569f7eaa904295</em> to <em>0ee7f20ff6a003d52224c84510e667483c6ddc2b</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=5b76872db43bcba752a6d72d5801cb3e0f8243fb"><span class="icon"></span>5b76872</a></td><td><code>Merge branch 'develop' into needswork/8734</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ca6a22176283d83b78998c17ad7a1335f390da40"><span class="icon"></span>ca6a221</a></td><td><code>factor out missing assumption error handling; filter _SAGE_VAR_</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3b7e91613a1cc43009d8f0fd89cf1f40e5ed452f"><span class="icon"></span>3b7e916</a></td><td><code>fix typo leading to error</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=0ee7f20ff6a003d52224c84510e667483c6ddc2b"><span class="icon"></span>0ee7f20</a></td><td><code>two further maxima calls adapted; two more doctests fixed</code>
</td></tr></table>
TicketRalf StephanSat, 22 Mar 2014 15:31:01 GMTwork_issues, author changed; reviewer deleted
https://trac.sagemath.org/ticket/8734#comment:16
https://trac.sagemath.org/ticket/8734#comment:16
<ul>
<li><strong>reviewer</strong>
<em>Ralf Stephan</em> deleted
</li>
<li><strong>work_issues</strong>
changed from <em>rebase</em> to <em>fix doctest problems</em>
</li>
<li><strong>author</strong>
changed from <em>Jason Grout</em> to <em>Jason Grout, Ralf Stephan</em>
</li>
</ul>
<p>
This fixes the errors in symbolic. More to come...
</p>
TicketKarl-Dieter CrismanSat, 22 Mar 2014 15:59:13 GMT
https://trac.sagemath.org/ticket/8734#comment:17
https://trac.sagemath.org/ticket/8734#comment:17
<p>
Thanks for working on this! This could also then eventually lead to a way to deal with the extra variables that come from e.g. differential equation solvers in Maxima - if we picked up a non-existent (in Sage) variable, we could perhaps do something with it. (There are tickets about this out there, so I won't repeat that discussion.)
</p>
TicketgitMon, 24 Mar 2014 15:10:48 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:18
https://trac.sagemath.org/ticket/8734#comment:18
<ul>
<li><strong>commit</strong>
changed from <em>0ee7f20ff6a003d52224c84510e667483c6ddc2b</em> to <em>a5f27eed1dfb83e1c979ecb23ad2854ccd00400b</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b1cd2bfb21798eb7a4f5ffa28e4c7635dc9471dc"><span class="icon"></span>b1cd2bf</a></td><td><code>fix and generalize missing_assumption(); resp. doctest adaptions</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b4df332a957c9ecd211e535f82fa6c62d3f1fc62"><span class="icon"></span>b4df332</a></td><td><code>Merge branch 'develop' into needswork/8734</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a5f27eed1dfb83e1c979ecb23ad2854ccd00400b"><span class="icon"></span>a5f27ee</a></td><td><code>add even more cases to add _SAGE_VAR_; fix doctests</code>
</td></tr></table>
TicketRalf StephanMon, 24 Mar 2014 15:13:17 GMTstatus changed
https://trac.sagemath.org/ticket/8734#comment:19
https://trac.sagemath.org/ticket/8734#comment:19
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
This fixes the last doctests, mainly by handling the desolver entry points.
</p>
TicketVolker BraunMon, 14 Apr 2014 22:49:09 GMT
https://trac.sagemath.org/ticket/8734#comment:20
https://trac.sagemath.org/ticket/8734#comment:20
<p>
Merge conflict, please merge in the latest beta
</p>
TicketRalf StephanTue, 15 Apr 2014 06:17:25 GMTcommit, branch changed; work_issues deleted
https://trac.sagemath.org/ticket/8734#comment:21
https://trac.sagemath.org/ticket/8734#comment:21
<ul>
<li><strong>commit</strong>
changed from <em>a5f27eed1dfb83e1c979ecb23ad2854ccd00400b</em> to <em>f00cd8d5dc695dee49f59164251772db46bf48c3</em>
</li>
<li><strong>work_issues</strong>
<em>fix doctest problems</em> deleted
</li>
<li><strong>branch</strong>
changed from <em>u/rws/ticket/8734</em> to <em>u/rws/ticket/8734-1</em>
</li>
</ul>
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3d90c49bc248680a90f2788697311e65f297234f"><span class="icon"></span>3d90c49</a></td><td><code>Trac #8734: Make sage variables unique in maxima.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a6cbf80ccf71dac48cc8cd352cb39bd913ecf651"><span class="icon"></span>a6cbf80</a></td><td><code>factor out missing assumption error handling; filter _SAGE_VAR_</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3f24835a9c82aeccde6135249bf1557cb9ced788"><span class="icon"></span>3f24835</a></td><td><code>fix typo leading to error</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=f36c6b52d6795d81b9c7918926e9a92ac5220cdd"><span class="icon"></span>f36c6b5</a></td><td><code>two further maxima calls adapted; two more doctests fixed</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ced268db4e1e150b453297d05bc42ab31abbeba5"><span class="icon"></span>ced268d</a></td><td><code>fix and generalize missing_assumption(); resp. doctest adaptions</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a74ec004d982cebddcd5cbb6d489f1b5978451c7"><span class="icon"></span>a74ec00</a></td><td><code>add even more cases to add _SAGE_VAR_; fix doctests</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=f00cd8d5dc695dee49f59164251772db46bf48c3"><span class="icon"></span>f00cd8d</a></td><td><code>Merge branch 'rev/8734' into tmp</code>
</td></tr></table>
TicketVolker BraunTue, 15 Apr 2014 09:45:47 GMTreviewer set
https://trac.sagemath.org/ticket/8734#comment:22
https://trac.sagemath.org/ticket/8734#comment:22
<ul>
<li><strong>reviewer</strong>
set to <em>Volker Braun</em>
</li>
</ul>
TicketVolker BraunTue, 15 Apr 2014 11:02:22 GMT
https://trac.sagemath.org/ticket/8734#comment:23
https://trac.sagemath.org/ticket/8734#comment:23
<pre class="wiki">sage -t --long src/sage/functions/bessel.py # 3 doctests failed
sage -t --long src/sage/interfaces/maxima_abstract.py # 16 doctests failed
sage -t --long src/sage/tests/french_book/recequadiff.py # 1 doctest failed
sage -t --long src/sage/interfaces/maxima.py # 2 doctests failed
sage -t --long src/doc/en/constructions/calculus.rst # 2 doctests failed
sage -t --long src/sage/interfaces/maxima_lib.py # 11 doctests failed
sage -t --long src/sage/functions/other.py # 4 doctests failed
sage -t --long src/sage/functions/orthogonal_polys.py # 5 doctests failed
sage -t --long src/sage/symbolic/expression_conversions.py # 2 doctests failed
sage -t --long src/sage/interfaces/interface.py # 10 doctests failed
</pre>
TicketVolker BraunTue, 15 Apr 2014 11:02:28 GMTstatus changed
https://trac.sagemath.org/ticket/8734#comment:24
https://trac.sagemath.org/ticket/8734#comment:24
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
TicketgitTue, 15 Apr 2014 16:44:24 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:25
https://trac.sagemath.org/ticket/8734#comment:25
<ul>
<li><strong>commit</strong>
changed from <em>f00cd8d5dc695dee49f59164251772db46bf48c3</em> to <em>be9367f966fc76ba6028b57ab4317530cfd1205e</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=4a570d870ac2d06f9562b612ce6685e68a0b000d"><span class="icon"></span>4a570d8</a></td><td><code>8734: more additions of _SAGE_VAR_ in doctests</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=be9367f966fc76ba6028b57ab4317530cfd1205e"><span class="icon"></span>be9367f</a></td><td><code>8734: one more maxima call adapted; more doctests fixed</code>
</td></tr></table>
TicketRalf StephanThu, 17 Apr 2014 08:50:59 GMT
https://trac.sagemath.org/ticket/8734#comment:26
https://trac.sagemath.org/ticket/8734#comment:26
<p>
Concerning the remaining doctests in <code>interfaces/maxima_abstract.py</code> I am at a loss at what to do. For example this
</p>
<pre class="wiki">sage: x,y = var('x,y'); f = maxima.function('x','sin(x)')
sage: type(f)
<class 'sage.interfaces.maxima.MaximaElementFunction'>
sage: g=maxima.cos(x)
sage: type(g)
<class 'sage.interfaces.maxima.MaximaElement'>
sage: f+g
cos(_SAGE_VAR_x)+sin(x)
sage: (f+g)(2)
cos(_SAGE_VAR_x)+sin(2)
</pre><p>
shows that while the <code>MaximaElementFunction f</code> has the variable <code>x</code> which is associated with <code>_SAGE_VAR_x</code> in Maxima (and calling the function works as expected), the <code>MaximaElement g</code> shows <code>_SAGE_VAR_x</code> which has of course no Maxima pendant (and calling the function bombs). Naively both should behave identically.
</p>
<pre class="wiki">sage: h=SR(maxima.cos(x))
sage: h
cos(x)
sage: h(2)
cos(2)
sage: f+h
cos(_SAGE_VAR_x)+sin(x)
sage: (f+h)(2)
cos(_SAGE_VAR_x)+sin(2)
</pre><p>
Moreover, if <code>g</code> gets converted to <code>SR</code> it behaves fine but when converted to <code>type(f)</code> by using it as rhs it gets <code>_SAGE_VAR_x</code> as parameter. What is the next step?
</p>
TicketRalf StephanFri, 18 Apr 2014 14:33:49 GMT
https://trac.sagemath.org/ticket/8734#comment:27
https://trac.sagemath.org/ticket/8734#comment:27
<p>
As far as I understand now, a <code>MaximaElementFunction</code> is a function defined within the Maxima process, and so <code>x</code> is the parameter name, not a registered variable. Thus the output <code>sin(x)</code> makes sense. OTOH any <code>_SAGE_VAR_x</code> that appears in a <code>MaximaElement</code> repr refers to a registered variable within Maxima that is associated with a registered Sage var named <code>x</code>. Let's interpret the above output in the light of this.
</p>
<blockquote class="citation">
<pre class="wiki">sage: f+g
cos(_SAGE_VAR_x)+sin(x)
</pre></blockquote>
<p>
Correct but can the user make sense of it?
</p>
<blockquote class="citation">
<pre class="wiki">sage: (f+g)(2)
cos(_SAGE_VAR_x)+sin(2)
</pre></blockquote>
<p>
That is not less correct than
</p>
<pre class="wiki">sage: (sin(x)+cos(y))(2)
cos(y) + sin(2)
</pre><p>
and it just seems to be another case of <code>SR._call_element_()</code> where is stated: "Note that you make get unexpected results when calling symbolic expressions and not explicitly giving the variables."
</p>
<p>
Moreover, it was a hack that this worked at all in the <code>MaximaAbstractElementFunction._add_()</code> doctests because
</p>
<pre class="wiki">sage: f = maxima.function('z','sin(z)')
sage: f
sin(z)
sage: f(2)
sin(2)
sage: (sin(z))(2)
---------------------------------------------------------------------------
NameError Traceback (most recent call last)
NameError: name 'z' is not defined
</pre>
TicketgitFri, 18 Apr 2014 14:57:40 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:28
https://trac.sagemath.org/ticket/8734#comment:28
<ul>
<li><strong>commit</strong>
changed from <em>be9367f966fc76ba6028b57ab4317530cfd1205e</em> to <em>06b952805ab10b7f30b4c06c1fcd487cbf1b7dce</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=1e9b54bca59e9b69cd10a08a60255012a90c4f01"><span class="icon"></span>1e9b54b</a></td><td><code>8734: more doctests fixed</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=06b952805ab10b7f30b4c06c1fcd487cbf1b7dce"><span class="icon"></span>06b9528</a></td><td><code>8734: fix doctests</code>
</td></tr></table>
TicketRalf StephanFri, 18 Apr 2014 14:58:21 GMTstatus changed
https://trac.sagemath.org/ticket/8734#comment:29
https://trac.sagemath.org/ticket/8734#comment:29
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketFor batch modificationsTue, 06 May 2014 15:20:58 GMTmilestone changed
https://trac.sagemath.org/ticket/8734#comment:30
https://trac.sagemath.org/ticket/8734#comment:30
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.2</em> to <em>sage-6.3</em>
</li>
</ul>
TicketgitMon, 26 May 2014 09:29:01 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:31
https://trac.sagemath.org/ticket/8734#comment:31
<ul>
<li><strong>commit</strong>
changed from <em>06b952805ab10b7f30b4c06c1fcd487cbf1b7dce</em> to <em>6fffc21fbfe9a72f40f51ba22efec6dd9ec28ac5</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=6fffc21fbfe9a72f40f51ba22efec6dd9ec28ac5"><span class="icon"></span>6fffc21</a></td><td><code>Merge branch 'develop' into t/8734/ticket/8734-1</code>
</td></tr></table>
TicketgitMon, 26 May 2014 13:57:02 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:32
https://trac.sagemath.org/ticket/8734#comment:32
<ul>
<li><strong>commit</strong>
changed from <em>6fffc21fbfe9a72f40f51ba22efec6dd9ec28ac5</em> to <em>3d74cad55ddc6d1a0c64356a59c88cdac1a6f2d9</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3d74cad55ddc6d1a0c64356a59c88cdac1a6f2d9"><span class="icon"></span>3d74cad</a></td><td><code>8734: rename private function, doctest it</code>
</td></tr></table>
TicketgitFri, 06 Jun 2014 13:23:27 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:33
https://trac.sagemath.org/ticket/8734#comment:33
<ul>
<li><strong>commit</strong>
changed from <em>3d74cad55ddc6d1a0c64356a59c88cdac1a6f2d9</em> to <em>cec9681b1ebb379ed6eccec389981512acd527cb</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3180e7b9ed36763261d579fafcb9cc67f7a887e8"><span class="icon"></span>3180e7b</a></td><td><code>Merge branch 'develop' into t/8734/ticket/8734-1</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=cec9681b1ebb379ed6eccec389981512acd527cb"><span class="icon"></span>cec9681</a></td><td><code>8734: merge conflicts</code>
</td></tr></table>
TicketgitFri, 06 Jun 2014 13:50:34 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:34
https://trac.sagemath.org/ticket/8734#comment:34
<ul>
<li><strong>commit</strong>
changed from <em>cec9681b1ebb379ed6eccec389981512acd527cb</em> to <em>f70208835cb64305fc90524a9b67c5a86aed0c1e</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=f70208835cb64305fc90524a9b67c5a86aed0c1e"><span class="icon"></span>f702088</a></td><td><code>take back unrelated change</code>
</td></tr></table>
TicketKarl-Dieter CrismanFri, 06 Jun 2014 14:40:48 GMTreviewer changed
https://trac.sagemath.org/ticket/8734#comment:35
https://trac.sagemath.org/ticket/8734#comment:35
<ul>
<li><strong>reviewer</strong>
changed from <em>Volker Braun</em> to <em>Volker Braun, Paul Zimmerman, Karl-Dieter Crisman</em>
</li>
</ul>
<p>
Hi! Thanks for keeping at this. I'll try to look at it one final time soon - waiting for the latest beta to compile. In the meantime:
</p>
<p>
I like the renaming and doctesting of the missing assumption function, and I agree with your analysis of the Maxima 'parameters', as you call them - yes, those are completely separate from Sage. I would encourage you to be even more explicit than
</p>
<pre class="wiki">The parameter ``x`` is different from the symbolic variable::
</pre><p>
by saying something about it being a <em>Maxima</em> parameter versus <em>Sage</em> symbolic variable. Does that make sense?
</p>
<p>
Also, don't worry too much about the 'todo' in <code>doc/en/constructions/calculus.rst</code>. That is truly ancient, from the days where legendary heroes of yore managed to bring calculus functionality, despite awkward syntax, into an algebraic geometry program... meaning eventually that should be just rewritten to use "native syntax" or just folded into the calculus documentation.
</p>
TicketgitSat, 07 Jun 2014 05:32:43 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:36
https://trac.sagemath.org/ticket/8734#comment:36
<ul>
<li><strong>commit</strong>
changed from <em>f70208835cb64305fc90524a9b67c5a86aed0c1e</em> to <em>2d31fb12b7a2932624a4ba2b310582d190d70824</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=2d31fb12b7a2932624a4ba2b310582d190d70824"><span class="icon"></span>2d31fb1</a></td><td><code>8734: doc cosmetics</code>
</td></tr></table>
TicketKarl-Dieter CrismanTue, 17 Jun 2014 15:11:17 GMT
https://trac.sagemath.org/ticket/8734#comment:37
https://trac.sagemath.org/ticket/8734#comment:37
<p>
Okay, here are a few questions. I am pretty sure the answers are very straightforward, but I want to make sure it's clear - in case we might want to add a doctest, for instance.
</p>
<ul><li>Commit be9367 (where you add a try/except clause in <code>_create</code>) - what sort of situation is that catching? (Also, did you change all of the doctests with assumptions, or leave a few just so people see what the full form looks like?)
</li><li>Commit ced268 (where you generalized the missing assumptions) - what situation is that additionally catching? Was that a case of Maxima asking questions which we didn't catch (and hence doctest) before?
</li><li>I assume you are more than happy with Jason's original patch doing the basic functionality, right?
</li></ul><p>
The only part I haven't had a chance to really dig into is the differential equations changes (Commit a74ec0). I assume there isn't an obvious more modular way to deal with those :( but I don't have time currently to go through this in detail. My main issue (not really a concern, just my lack of being convinced yet) is just seeing that we really aren't over-replacing or under-replacing in that.
</p>
<p>
So these are pretty minor things and hopefully I'll be able to find some time for the DE part (or someone else will!) and we'll be on our way! It certainly looks like you were VERY thorough in finding places that might cause trouble. The difficulty is that one might miss some places it is needed because in the absence of adding <code>_SAGE_VAR_</code> things should still work, so one might not know if we missed one. Thanks!
</p>
TicketRalf StephanTue, 17 Jun 2014 15:45:47 GMT
https://trac.sagemath.org/ticket/8734#comment:38
https://trac.sagemath.org/ticket/8734#comment:38
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8734#comment:37" title="Comment 37">kcrisman</a>:
</p>
<blockquote class="citation">
<p>
Okay, here are a few questions. I am pretty sure the answers are very straightforward, but I want to make sure it's clear - in case we might want to add a doctest, for instance.
</p>
</blockquote>
<p>
Sure, as long as you remember "The perfect is the enemy of the good."
</p>
<blockquote class="citation">
<ul><li>Commit be9367 (where you add a try/except clause in <code>_create</code>) - what sort of situation is that catching?
</li></ul></blockquote>
<p>
There are several places where maxima.eval() is called and where exceptions are thrown. This one was simply overlooked, so that change fixes an unreported bug.
</p>
<blockquote class="citation">
<p>
(Also, did you change all of the doctests with assumptions, or leave a few just so people see what the full form looks like?)
</p>
</blockquote>
<p>
Yes.
</p>
<blockquote class="citation">
<ul><li>Commit ced268 (where you generalized the missing assumptions) - what situation is that additionally catching? Was that a case of Maxima asking questions which we didn't catch (and hence doctest) before?
</li></ul></blockquote>
<p>
No, it reduces code duplication. Recommended reading: <a class="ext-link" href="https://en.wikipedia.org/wiki/Code_refactoring"><span class="icon"></span>https://en.wikipedia.org/wiki/Code_refactoring</a>
</p>
<blockquote class="citation">
<ul><li>I assume you are more than happy with Jason's original patch doing the basic functionality, right?
</li></ul></blockquote>
<p>
Yes and no. A lot was missing.
</p>
<blockquote class="citation">
<p>
...
So these are pretty minor things and hopefully I'll be able to find some time for the DE part (or someone else will!) and we'll be on our way! It certainly looks like you were VERY thorough in finding places that might cause trouble. The difficulty is that one might miss some places it is needed because in the absence of adding <code>_SAGE_VAR_</code> things should still work, so one might not know if we missed one. Thanks!
</p>
</blockquote>
<p>
I would assume this is caught by all those doctests using maxima.
</p>
TicketgitTue, 17 Jun 2014 15:47:31 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:39
https://trac.sagemath.org/ticket/8734#comment:39
<ul>
<li><strong>commit</strong>
changed from <em>2d31fb12b7a2932624a4ba2b310582d190d70824</em> to <em>2c68026320ba3c516cd94fa1721e886ecd732804</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=2c68026320ba3c516cd94fa1721e886ecd732804"><span class="icon"></span>2c68026</a></td><td><code>8734: adapt doctest to recent change of output</code>
</td></tr></table>
TicketRalf StephanTue, 17 Jun 2014 15:48:17 GMT
https://trac.sagemath.org/ticket/8734#comment:40
https://trac.sagemath.org/ticket/8734#comment:40
<p>
The output changed recently and I forgot to fix one doctest.
</p>
TicketKarl-Dieter CrismanTue, 17 Jun 2014 16:04:10 GMT
https://trac.sagemath.org/ticket/8734#comment:41
https://trac.sagemath.org/ticket/8734#comment:41
<blockquote class="citation">
<blockquote class="citation">
<p>
Okay, here are a few questions. I am pretty sure the answers are very straightforward, but I want to make sure it's clear - in case we might want to add a doctest, for instance.
</p>
</blockquote>
<p>
Sure, as long as you remember "The perfect is the enemy of the good."
</p>
</blockquote>
<p>
Of course!
</p>
<blockquote class="citation">
<blockquote class="citation">
<ul><li>Commit be9367 (where you add a try/except clause in <code>_create</code>) - what sort of situation is that catching?
</li></ul></blockquote>
<p>
There are several places where maxima.eval() is called and where exceptions are thrown. This one was simply overlooked, so that change fixes an unreported bug.
</p>
</blockquote>
<p>
Hmm, okay. I don't see how <code>_create</code> could have asked for an evaluation of this type, but I suppose.
</p>
<blockquote class="citation">
<blockquote class="citation">
<ul><li>Commit ced268 (where you generalized the missing assumptions) - what situation is that additionally catching? Was that a case of Maxima asking questions which we didn't catch (and hence doctest) before?
</li></ul></blockquote>
<p>
No, it reduces code duplication. Recommended reading: <a class="ext-link" href="https://en.wikipedia.org/wiki/Code_refactoring"><span class="icon"></span>https://en.wikipedia.org/wiki/Code_refactoring</a>
</p>
</blockquote>
<p>
I'm not referring to that; in fact, I fully agreed with that strategy in earlier comments. My question is specifically about <code>jj=2</code> - since usually <code>jj=3</code> seems to be the old case. We should test a new branch, which this appears to be (though it may just be something obvious I'm not seeing).
</p>
<blockquote class="citation">
<blockquote class="citation">
<ul><li>I assume you are more than happy with Jason's original patch doing the basic functionality, right?
</li></ul></blockquote>
<p>
Yes and no. A lot was missing.
</p>
</blockquote>
<p>
I mean in terms of reviewing that for the <em>basic</em> functionality for proper conversion. Naturally you provided a huge amount of missing stuff!
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
So these are pretty minor things and hopefully I'll be able to find some time for the DE part (or someone else will!) and we'll be on our way! It certainly looks like you were VERY thorough in finding places that might cause trouble. The difficulty is that one might miss some places it is needed because in the absence of adding <code>_SAGE_VAR_</code> things should still work, so one might not know if we missed one. Thanks!
</p>
</blockquote>
<p>
I would assume this is caught by all those doctests using maxima.
</p>
</blockquote>
<p>
That's my point - they may NOT catch a missing one, since we only remove things via search-and-replace-with-empty-string, and everything worked before.
</p>
TicketRalf StephanWed, 18 Jun 2014 08:39:13 GMT
https://trac.sagemath.org/ticket/8734#comment:42
https://trac.sagemath.org/ticket/8734#comment:42
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8734#comment:41" title="Comment 41">kcrisman</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<blockquote class="citation">
<ul><li>Commit be9367 (where you add a try/except clause in <code>_create</code>) - what sort of situation is that catching?
</li></ul></blockquote>
<p>
There are several places where maxima.eval() is called and where exceptions are thrown. This one was simply overlooked, so that change fixes an unreported bug.
</p>
</blockquote>
<p>
Hmm, okay. I don't see how <code>_create</code> could have asked for an evaluation of this type, but I suppose.
</p>
</blockquote>
<p>
It is really easy to find it out yourself: just change <code>RuntimeError</code> to something different like <code>NotImplementedError</code>, doctest, and you'll get:
</p>
<pre class="wiki">sage -t src/sage/tests/french_book/recequadiff.py
**********************************************************************
File "src/sage/tests/french_book/recequadiff.py", line 247, in sage.tests.french_book.recequadiff
Failed example:
desolve(eq1,[f,x])
Expected:
Traceback (most recent call last):
...
TypeError: Computation failed ...
Is k positive, negative or zero?
Got:
<BLANKLINE>
Traceback (most recent call last):
File "/home/ralf/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 480, in _run
self.execute(example, compiled, test.globs)
File "/home/ralf/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 839, in execute
exec compiled in globs
File "<doctest sage.tests.french_book.recequadiff[64]>", line 1, in <module>
desolve(eq1,[f,x])
File "/home/ralf/sage/local/lib/python2.7/site-packages/sage/calculus/desolvers.py", line 436, in desolve
soln = P(cmd)
File "/home/ralf/sage/local/lib/python2.7/site-packages/sage/interfaces/interface.py", line 199, in __call__
return cls(self, x, name=name)
File "/home/ralf/sage/local/lib/python2.7/site-packages/sage/interfaces/interface.py", line 626, in __init__
raise TypeError(x)
TypeError: ECL says: Maxima asks: Is _SAGE_VAR_k positive, negative or zero?
</pre><blockquote class="citation">
<blockquote class="citation">
<blockquote class="citation">
<ul><li>Commit ced268 (where you generalized the missing assumptions) - what situation is that additionally catching? Was that a case of Maxima asking questions which we didn't catch (and hence doctest) before?
</li></ul></blockquote>
<p>
No, it reduces code duplication. Recommended reading: <a class="ext-link" href="https://en.wikipedia.org/wiki/Code_refactoring"><span class="icon"></span>https://en.wikipedia.org/wiki/Code_refactoring</a>
</p>
</blockquote>
<p>
I'm not referring to that; in fact, I fully agreed with that strategy in earlier comments. My question is specifically about <code>jj=2</code> - since usually <code>jj=3</code> seems to be the old case. We should test a new branch, which this appears to be (though it may just be something obvious I'm not seeing).
</p>
</blockquote>
<p>
There were cases where there were two spaces in the output after 'Is '.
</p>
<blockquote class="citation">
<blockquote class="citation">
<blockquote class="citation">
<ul><li>I assume you are more than happy with Jason's original patch doing the basic functionality, right?
</li></ul></blockquote>
<p>
Yes and no. A lot was missing.
</p>
</blockquote>
<p>
I mean in terms of reviewing that for the <em>basic</em> functionality for proper conversion. Naturally you provided a huge amount of missing stuff!
</p>
</blockquote>
<p>
I think his ansatz was what I would have done, too.
</p>
<blockquote class="citation">
<blockquote class="citation">
<blockquote class="citation">
<p>
So these are pretty minor things and hopefully I'll be able to find some time for the DE part (or someone else will!) and we'll be on our way! It certainly looks like you were VERY thorough in finding places that might cause trouble. The difficulty is that one might miss some places it is needed because in the absence of adding <code>_SAGE_VAR_</code> things should still work, so one might not know if we missed one. Thanks!
</p>
</blockquote>
<p>
I would assume this is caught by all those doctests using maxima.
</p>
</blockquote>
<p>
That's my point - they may NOT catch a missing one, since we only remove things via search-and-replace-with-empty-string, and everything worked before.
</p>
</blockquote>
<p>
Can you please clarify: "one might miss some places it is needed"---what's the it here?
</p>
TicketKarl-Dieter CrismanWed, 18 Jun 2014 15:26:35 GMT
https://trac.sagemath.org/ticket/8734#comment:43
https://trac.sagemath.org/ticket/8734#comment:43
<blockquote class="citation">
<p>
There were cases where there were two spaces in the output after 'Is '.
</p>
</blockquote>
<p>
Annoying.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
That's my point - they may NOT catch a missing one, since we only remove things via search-and-replace-with-empty-string, and everything worked before.
</p>
</blockquote>
<p>
Can you please clarify: "one might miss some places it is needed"---what's the it here?
</p>
</blockquote>
<p>
What I mean is that if one had missed a place where we should have added <code>SAGE_VAR</code> but didn't, we might not know because the result would not need <code>SAGE_VAR</code> stripped from it, so it would perhaps still work. But I'm not overly concerned about this ... because it would still work.
</p>
<hr />
<p>
Anyway, if someone reviews the diffeq part before me (I won't have time before next week), that is all I think still needs review. This will be great to finally have in!
</p>
TicketNils BruinWed, 18 Jun 2014 20:47:28 GMT
https://trac.sagemath.org/ticket/8734#comment:44
https://trac.sagemath.org/ticket/8734#comment:44
<p>
For the
</p>
<pre class="wiki">try:
....
except ... as error:
...
raise error
</pre><p>
in maxima_lib.py (and possibly elsewhere):
It's *much* better to reraise an error with a bare <code>raise</code> rather than <code>raise error</code>, since the bare raise will leave the original traceback intact, whereas the <code>raise error</code> will create a new traceback, obscuring the actual source of the error.
</p>
TicketRalf StephanThu, 19 Jun 2014 09:20:53 GMT
https://trac.sagemath.org/ticket/8734#comment:45
https://trac.sagemath.org/ticket/8734#comment:45
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8734#comment:44" title="Comment 44">nbruin</a>:
</p>
<blockquote class="citation">
<p>
For the
</p>
<pre class="wiki">try:
....
except ... as error:
...
raise error
</pre><p>
in maxima_lib.py (and possibly elsewhere):
It's *much* better to reraise an error with a bare <code>raise</code> rather than <code>raise error</code>, since the bare raise will leave the original traceback intact, whereas the <code>raise error</code> will create a new traceback, obscuring the actual source of the error.
</p>
</blockquote>
<p>
In this case, yes. Else, are you requesting to rethrow the <code>RuntimeError</code> from Maxima in all cases as <code>RuntimeError</code>? I'm asking because it appears that preserving the stacktrace and throwing the more specific <code>ValueError</code> appears possible:
<a class="ext-link" href="http://www.gossamer-threads.com/lists/python/python/947257"><span class="icon"></span>http://www.gossamer-threads.com/lists/python/python/947257</a>
</p>
<pre class="wiki">In Python 3 you could chain the exceptions with:
except Exception as e:
raise CustomException() from e
There is no such syntax in Python 2, but you could manually store and
retrieve the __cause__ and __traceback__ attributes similarly to the
way Python 3 does it. See PEP 3134 for full details.
</pre><p>
<a class="ext-link" href="http://legacy.python.org/dev/peps/pep-3134/"><span class="icon"></span>http://legacy.python.org/dev/peps/pep-3134/</a>
</p>
<pre class="wiki">Sometimes it can be useful for an exception handler to intentionally
re-raise an exception, either to provide extra information or to
translate an exception to another type. ...
</pre>
TicketgitThu, 19 Jun 2014 09:22:37 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:46
https://trac.sagemath.org/ticket/8734#comment:46
<ul>
<li><strong>commit</strong>
changed from <em>2c68026320ba3c516cd94fa1721e886ecd732804</em> to <em>793bb058a06b3dfb31c6029f6c30960d1dee8cc7</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=793bb058a06b3dfb31c6029f6c30960d1dee8cc7"><span class="icon"></span>793bb05</a></td><td><code>8734: reraise in "other" cases with bare "raise"</code>
</td></tr></table>
TicketNils BruinThu, 19 Jun 2014 13:19:29 GMT
https://trac.sagemath.org/ticket/8734#comment:47
https://trac.sagemath.org/ticket/8734#comment:47
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8734#comment:45" title="Comment 45">rws</a>:
</p>
<blockquote class="citation">
<p>
In this case, yes. Else, are you requesting to rethrow the <code>RuntimeError</code> from Maxima in all cases as <code>RuntimeError</code>? I'm asking because it appears that preserving the stacktrace and throwing the more specific <code>ValueError</code> appears possible:
</p>
</blockquote>
<p>
Good to know! I don't have a strong opinion what type of error to return. However, without further inspection, I don't think you would know if the error is more appropriately a <code>ValueError</code>. You could get <code>RuntimeError("ECL says: I'm tired")</code>for as far as you know at this point. So my guess is that it's not worth trying to do something with the error type.
</p>
TicketRalf StephanFri, 20 Jun 2014 07:28:18 GMTdependencies set
https://trac.sagemath.org/ticket/8734#comment:48
https://trac.sagemath.org/ticket/8734#comment:48
<ul>
<li><strong>dependencies</strong>
set to <em>#15530</em>
</li>
</ul>
<p>
All the following is modulo my Python newbieness.
</p>
<p>
What I wrote about legal tricks to preserve the traceback would work IF Sage was purely Python-2 or Python-3. With the momentary transition to Py-3, the form <code>raise Error, message, traceback</code> of the raise command is considered a bug. OTOH there is no way to supply two arguments in parentheses. Finally, the form <code>raise Error from previous_error</code> is only possible with Py-3.
</p>
<p>
Secondly, to ask interactively for a missing assumption a message has to be given in the terminal different from the error message tied to the original <code>RuntimeError</code>. So, simply re-raising is not an option if you do not like <code>RuntimeError: ECL says: Maxima asks: Is _SAGE_VAR_a an integer?</code>.
</p>
<p>
So, in my opinion, unless someone comes up with a better solution, this ticket will have to wait until Sage is Py-3.
</p>
TicketNils BruinFri, 20 Jun 2014 12:54:30 GMT
https://trac.sagemath.org/ticket/8734#comment:49
https://trac.sagemath.org/ticket/8734#comment:49
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8734#comment:48" title="Comment 48">rws</a>:
</p>
<blockquote class="citation">
<p>
Secondly, to ask interactively for a missing assumption a message has to be given in the terminal different from the error message tied to the original <code>RuntimeError</code>. So, simply re-raising is not an option if you do not like <code>RuntimeError: ECL says: Maxima asks: Is _SAGE_VAR_a an integer?</code>.
</p>
</blockquote>
<p>
In that case we know exactly what happened and the original traceback doesn't have to be kept. Raising a a fresh exception with a fresh traceback should be fine. It's when you find that the <code>RuntimeError</code> you've just caught is *not* the one you expected that the original traceback is valuable. And in that case you probably don't want to mess with the exception object itself either, so a bare <code>raise</code> should do the trick. The scenario of changing the error object but keeping the original traceback should be quite rare.
</p>
<blockquote class="citation">
<p>
So, in my opinion, unless someone comes up with a better solution, this ticket will have to wait until Sage is Py-3.
</p>
</blockquote>
<p>
which may be a very long time. It nice to do things in a Py2/Py3 compatible way if possible (which can be done here, I think), but if not we'll just have to fix it if/when sage transitions from Py2 to Py3.
</p>
TicketKarl-Dieter CrismanFri, 20 Jun 2014 13:08:40 GMT
https://trac.sagemath.org/ticket/8734#comment:50
https://trac.sagemath.org/ticket/8734#comment:50
<blockquote class="citation">
<blockquote class="citation">
<p>
So, in my opinion, unless someone comes up with a better solution, this ticket will have to wait until Sage is Py-3.
</p>
</blockquote>
<p>
which may be a very long time.
</p>
</blockquote>
<p>
Indeed.
</p>
<blockquote class="citation">
<p>
It nice to do things in a Py2/Py3 compatible way if possible (which can be done here, I think), but if not we'll just have to fix it if/when sage transitions from Py2 to Py3.
</p>
</blockquote>
<p>
I'm not quite clear on why this is the case. I thought you said above to not let the perfect be the enemy of the good ;-) It seems like just raising the original exception is a good compromise here; at the very least it should give <em>some</em> information, right?
</p>
TicketRalf StephanFri, 20 Jun 2014 13:46:25 GMT
https://trac.sagemath.org/ticket/8734#comment:51
https://trac.sagemath.org/ticket/8734#comment:51
<p>
I agree I was the perfectionist this time so, with the bare raise patch already uploaded, it remains for me to thank you and ask for positive review.
</p>
TicketKarl-Dieter CrismanFri, 20 Jun 2014 17:20:36 GMT
https://trac.sagemath.org/ticket/8734#comment:52
https://trac.sagemath.org/ticket/8734#comment:52
<p>
Again, for me, all is positive review except the diffeq commit, and that only because I haven't had time to look carefully at it, not because I suspect any problems - it looks quite thorough.
</p>
TicketRalf StephanSun, 22 Jun 2014 07:24:02 GMTdependencies deleted
https://trac.sagemath.org/ticket/8734#comment:53
https://trac.sagemath.org/ticket/8734#comment:53
<ul>
<li><strong>dependencies</strong>
<em>#15530</em> deleted
</li>
</ul>
TicketKarl-Dieter CrismanThu, 26 Jun 2014 14:06:59 GMT
https://trac.sagemath.org/ticket/8734#comment:54
https://trac.sagemath.org/ticket/8734#comment:54
<p>
Okay, I have a few questions about <a class="ext-link" href="http://git.sagemath.org/sage.git/diff/src/sage/calculus/desolvers.py?id=610fb79bfc9fde84dea1200fdfcd4bceb25bd77f"><span class="icon"></span>the ode changes</a>. I assume the answer to all of them is "it would have been even messier any other way", but I just wanted to check.
</p>
<ul><li>In some of the sanitizing functions, you replace things like <code>'_SAGE_VAR_f</code> with <code>f</code>, but in others you only replace the independent variable that way. Is that because of specific examples that didn't work, or was the context different, or... ?
</li><li>I'm wondering whether the Sage translation would have just taken care of this in <code>soln.sage()</code>, but I guess it didn't. Was there any possible change to the translation that could have done this, rather than getting into the ode wrapper code directly (which makes it harder to read)? For instance, in <code>desolve_laplace</code> we convert the <code>de</code> to Maxima (presumably adding <code>SAGE_VAR</code>, add another <code>SAGE_VAR</code> from <code>f(x)</code> to <code>f(_SAGE_VAR_x)</code> (I think), and then proceed to remove only the <code>SAGE_VAR</code> from the <em>de</em>pendent variable. So... that part isn't taken care of by the translation, but the independent variable still somehow is translated back correctly, but not forward within <code>de0=de._maxima_()</code>? Yet in the <code>rk4</code> types this isn't a problem, apparently.
</li><li>We should probably just remove <code>desolve_system_strings</code>, see <a class="closed ticket" href="https://trac.sagemath.org/ticket/8132" title="#8132: defect: fix documentation related to ODE solvers (closed: fixed)">#8132</a> where it was first said to be obsolete, and it hasn't been in the global namespace since before 2010. That is pretty much equivalent to a deprecation to me. However, we should keep any non-overlapping examples - so maybe removal should be another ticket...
</li></ul><p>
But it seems good, assuming I didn't miss any tests that fail...
</p>
TicketKarl-Dieter CrismanThu, 26 Jun 2014 14:54:36 GMT
https://trac.sagemath.org/ticket/8734#comment:55
https://trac.sagemath.org/ticket/8734#comment:55
<blockquote class="citation">
<p>
But it seems good, assuming I didn't miss any tests that fail...
</p>
</blockquote>
<p>
I didn't. So as long as you give the answers I expect, we are all set here.
</p>
TicketRalf StephanFri, 27 Jun 2014 14:28:48 GMT
https://trac.sagemath.org/ticket/8734#comment:56
https://trac.sagemath.org/ticket/8734#comment:56
<p>
Working from the bottom up ...
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8734#comment:54" title="Comment 54">kcrisman</a>:
</p>
<blockquote class="citation">
<ul><li>We should probably just remove <code>desolve_system_strings</code>, see <a class="closed ticket" href="https://trac.sagemath.org/ticket/8132" title="#8132: defect: fix documentation related to ODE solvers (closed: fixed)">#8132</a> where it was first said to be obsolete, and it hasn't been in the global namespace since before 2010. That is pretty much equivalent to a deprecation to me. However, we should keep any non-overlapping examples - so maybe removal should be another ticket...
</li></ul></blockquote>
<p>
This is now <a class="closed ticket" href="https://trac.sagemath.org/ticket/16568" title="#16568: task: remove desolve_system_strings() (closed: fixed)">#16568</a>.
</p>
TicketgitSun, 29 Jun 2014 08:29:45 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:57
https://trac.sagemath.org/ticket/8734#comment:57
<ul>
<li><strong>commit</strong>
changed from <em>793bb058a06b3dfb31c6029f6c30960d1dee8cc7</em> to <em>a195985c0617bf14a5d644f5c065bea896340ce4</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a195985c0617bf14a5d644f5c065bea896340ce4"><span class="icon"></span>a195985</a></td><td><code>8734: remove superfluous code</code>
</td></tr></table>
TicketRalf StephanSun, 29 Jun 2014 08:50:14 GMT
https://trac.sagemath.org/ticket/8734#comment:58
https://trac.sagemath.org/ticket/8734#comment:58
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/8734#comment:54" title="Comment 54">kcrisman</a>:
</p>
<blockquote class="citation">
<ul><li>In some of the sanitizing functions, you replace things like <code>'_SAGE_VAR_f</code> with <code>f</code>, but in others you only replace the independent variable that way. Is that because of specific examples that didn't work, or was the context different, or... ?
</li></ul></blockquote>
<p>
It certainly was then because of specific examples. The removal of the code I think you mean makes no difference now, however.
</p>
<blockquote class="citation">
<ul><li>I'm wondering whether the Sage translation would have just taken care of this in <code>soln.sage()</code>, but I guess it didn't. Was there any possible change to the translation that could have done this, rather than getting into the ode wrapper code directly (which makes it harder to read)? For instance, in <code>desolve_laplace</code> we convert the <code>de</code> to Maxima (presumably adding <code>SAGE_VAR</code>, add another <code>SAGE_VAR</code> from <code>f(x)</code> to <code>f(_SAGE_VAR_x)</code> (I think), and then proceed to remove only the <code>SAGE_VAR</code> from the <em>de</em>pendent variable. So... that part isn't taken care of by the translation, but the independent variable still somehow is translated back correctly, but not forward within <code>de0=de._maxima_()</code>? Yet in the <code>rk4</code> types this isn't a problem, apparently.
</li></ul></blockquote>
<p>
With the above removal of superfluous code I don't see any different behaviour in the three functions <code>laplace/system/rk4</code>. There is always the marking of the dependent var to prepare for the call to solve. You cannot remove this because <code>maxima(cmd)</code> does no translation, it's the low-level call. The translation has to be done here, as far as I understand.
</p>
TicketKarl-Dieter CrismanWed, 02 Jul 2014 02:55:46 GMT
https://trac.sagemath.org/ticket/8734#comment:59
https://trac.sagemath.org/ticket/8734#comment:59
<blockquote class="citation">
<p>
With the above removal of superfluous code I don't see any different behaviour in the three functions <code>laplace/system/rk4</code>. There is always the marking of the dependent var to prepare for the call to solve. You cannot remove this because <code>maxima(cmd)</code> does no translation, it's the low-level call. The translation has to be done here, as far as I understand.
</p>
</blockquote>
<p>
Again, I'm just confused because in <code>laplace/rk4</code> the dependent variable gets <code>_SAGE_VAR_</code> but in <code>desolve</code> it doesn't. Maybe this isn't important, though. And is the last change in <code>desolve</code> ok because putting things into Maxima takes care of the <code>_SAGE_VAR_</code> already?
</p>
TicketKarl-Dieter CrismanWed, 02 Jul 2014 12:52:21 GMT
https://trac.sagemath.org/ticket/8734#comment:60
https://trac.sagemath.org/ticket/8734#comment:60
<p>
Passes all tests.
</p>
TicketRalf StephanWed, 02 Jul 2014 15:13:04 GMT
https://trac.sagemath.org/ticket/8734#comment:61
https://trac.sagemath.org/ticket/8734#comment:61
<p>
After studying this in detail, the reason why the last change makes no difference is the following: in <code>desolve</code> and <code>desolve_laplace</code> the translation to Maxima is applied several times using <code>maxima()</code> and <code>P()</code> (which is the parent of the first translation result, i.e. of a Maxima expression). <code>P()</code> is also applied to <code>dvar.operator()</code> resulting in <code>dvar_str</code> which already has <code>_SAGE_VAR_</code> and is template for sanitization. Appending <code>_SAGE_VAR_</code> to <code>dvar_str</code> and replacing it thus was useless.
</p>
<p>
In <code>desolve_rk4</code> the <code>cmd</code> is constructed from two parts: the <code>de0</code> that gets translated via <code>._maxima()_</code> and the construction via string concatenation. So, there is no difference between <code>desolve</code> and <code>desolve_rk4</code> because <code>desolve</code> gets <code>_SAGE_VAR_</code> everywhere and <code>desolve_rk4</code> too.
</p>
TicketKarl-Dieter CrismanWed, 02 Jul 2014 15:45:37 GMTstatus changed
https://trac.sagemath.org/ticket/8734#comment:62
https://trac.sagemath.org/ticket/8734#comment:62
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Let's make it so, then. I was kind of suspecting there were extra <code>_SAGE_VAR_</code>s.
</p>
TicketVolker BraunThu, 03 Jul 2014 02:07:16 GMTstatus changed
https://trac.sagemath.org/ticket/8734#comment:63
https://trac.sagemath.org/ticket/8734#comment:63
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
conflicts with <a class="closed ticket" href="https://trac.sagemath.org/ticket/6882" title="#6882: defect: bugs in conversion of variable names from Maxima to Sage (closed: fixed)">#6882</a>
</p>
TicketgitThu, 03 Jul 2014 06:12:06 GMTcommit changed
https://trac.sagemath.org/ticket/8734#comment:64
https://trac.sagemath.org/ticket/8734#comment:64
<ul>
<li><strong>commit</strong>
changed from <em>a195985c0617bf14a5d644f5c065bea896340ce4</em> to <em>7a6696b714e638461cf4f77adbb65ecf413a2e4e</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=65107d4bac54adbceb615c7bdb0af6f4ca30d93c"><span class="icon"></span>65107d4</a></td><td><code>Merge branch 'develop' into t/8734/ticket/8734-1</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=4e0738355f228ea068bea927f3025dcccb955b1c"><span class="icon"></span>4e07383</a></td><td><code>6882: add rules for e, i, I</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e5a53435bde170e774c4e1dafba8f333a9653330"><span class="icon"></span>e5a5343</a></td><td><code>6882: do word search/replace for symtable keys</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=4c1b0eb5d6ff4b2b95487440c27c9879edb0131e"><span class="icon"></span>4c1b0eb</a></td><td><code>6882: correction to previous commit</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=518de3e62533cd209997107f903192f1a31d118c"><span class="icon"></span>518de3e</a></td><td><code>6882: add symable rules for e,i,I; fix maxima_var; add doctests</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=7a6696b714e638461cf4f77adbb65ecf413a2e4e"><span class="icon"></span>7a6696b</a></td><td><code>Merge branch 'u/rws/bugs_in_conversion_of_variable_names_from_maxima_to_sage' of trac.sagemath.org:sage into t/8734/ticket/8734-1</code>
</td></tr></table>
TicketRalf StephanThu, 03 Jul 2014 06:17:25 GMTstatus changed; dependencies set
https://trac.sagemath.org/ticket/8734#comment:65
https://trac.sagemath.org/ticket/8734#comment:65
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>positive_review</em>
</li>
<li><strong>dependencies</strong>
set to <em>#6882</em>
</li>
</ul>
<p>
Since the reviewed <a class="closed ticket" href="https://trac.sagemath.org/ticket/6882" title="#6882: defect: bugs in conversion of variable names from Maxima to Sage (closed: fixed)">#6882</a> is orthogonal I set positive again. Thanks Karl-Dieter for this review! I would be happy if you could have a look at <a class="closed ticket" href="https://trac.sagemath.org/ticket/2516" title="#2516: enhancement: generalized hypergeometric functions should be implemented (closed: fixed)">#2516</a> too.
</p>
TicketPaul ZimmermannThu, 03 Jul 2014 07:07:40 GMTreviewer changed
https://trac.sagemath.org/ticket/8734#comment:66
https://trac.sagemath.org/ticket/8734#comment:66
<ul>
<li><strong>reviewer</strong>
changed from <em>Volker Braun, Paul Zimmerman, Karl-Dieter Crisman</em> to <em>Volker Braun, Paul Zimmermann, Karl-Dieter Crisman</em>
</li>
</ul>
TicketKarl-Dieter CrismanThu, 03 Jul 2014 13:17:23 GMT
https://trac.sagemath.org/ticket/8734#comment:67
https://trac.sagemath.org/ticket/8734#comment:67
<blockquote class="citation">
<p>
I would be happy if you could have a look at <a class="closed ticket" href="https://trac.sagemath.org/ticket/2516" title="#2516: enhancement: generalized hypergeometric functions should be implemented (closed: fixed)">#2516</a> too.
</p>
</blockquote>
<p>
Sorry, that one I definitely like the idea of and have reviewed many similar ones in the past, but just don't have the time currently to review much new functionality carefully. :(
</p>
TicketVolker BraunSun, 06 Jul 2014 17:55:56 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/8734#comment:68
https://trac.sagemath.org/ticket/8734#comment:68
<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>branch</strong>
changed from <em>u/rws/ticket/8734-1</em> to <em>7a6696b714e638461cf4f77adbb65ecf413a2e4e</em>
</li>
</ul>
Ticket