Sage: Ticket #13735: Fixing a bug in sage.misc.misc.coeff_repr
https://trac.sagemath.org/ticket/13735
<p>
Currently, we have
</p>
<pre class="wiki">sage: alpha,beta,gamma=FreeAlgebra(ZZ,3,'alpha,beta,gamma').gens()
sage: latex(alpha-beta)
\alpha + \left(-1\right)\beta
</pre><p>
This patch turns this into
</p>
<pre class="wiki">sage: alpha,beta,gamma=FreeAlgebra(ZZ,3,'alpha,beta,gamma').gens()
sage: latex(alpha-beta)
\alpha - \beta
</pre><p>
<strong>Apply</strong> <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/13735/trac_13735_fix_repr_lincomb.patch" title="Attachment 'trac_13735_fix_repr_lincomb.patch' in Ticket #13735">trac_13735_fix_repr_lincomb.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/13735/trac_13735_fix_repr_lincomb.patch" title="Download"></a>
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/13735
Trac 1.1.6stumpc5Wed, 21 Nov 2012 11:11:45 GMTstatus changed
https://trac.sagemath.org/ticket/13735#comment:1
https://trac.sagemath.org/ticket/13735#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
Ticketstumpc5Wed, 21 Nov 2012 11:40:14 GMT
https://trac.sagemath.org/ticket/13735#comment:2
https://trac.sagemath.org/ticket/13735#comment:2
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/13735#comment:1" title="Comment 1">stumpc5</a>:
</p>
<p>
This one seems to not apply on 5.5.rc0 (while it does on 5.5.beta1). I am currently building rc0 and will fix this as soon as rc0 is ready on my machine.
</p>
Ticketstumpc5Wed, 21 Nov 2012 13:08:33 GMTattachment set
https://trac.sagemath.org/ticket/13735
https://trac.sagemath.org/ticket/13735
<ul>
<li><strong>attachment</strong>
set to <em>trac_13735-fix_bug_in_coef_repr-cs.patch</em>
</li>
</ul>
Ticketstumpc5Wed, 21 Nov 2012 13:08:57 GMT
https://trac.sagemath.org/ticket/13735#comment:3
https://trac.sagemath.org/ticket/13735#comment:3
<p>
apply trac_13735-fix_bug_in_coef_repr-cs.patch
</p>
TicketnovoseltWed, 21 Nov 2012 15:48:27 GMT
https://trac.sagemath.org/ticket/13735#comment:4
https://trac.sagemath.org/ticket/13735#comment:4
<p>
And what about <code>alpha - 2 * beta</code>? It does not look like the patch would improve this.
</p>
Ticketstumpc5Wed, 21 Nov 2012 15:52:10 GMT
https://trac.sagemath.org/ticket/13735#comment:5
https://trac.sagemath.org/ticket/13735#comment:5
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/13735#comment:4" title="Comment 4">novoselt</a>:
</p>
<blockquote class="citation">
<p>
And what about <code>alpha - 2 * beta</code>? It does not look like the patch would improve this.
</p>
</blockquote>
<p>
This is very right. And I think you're right that the patch should not only handle one particular case (which I was catching in a completely different context).
</p>
<p>
I will look into that...
</p>
Ticketstumpc5Wed, 21 Nov 2012 18:38:32 GMT
https://trac.sagemath.org/ticket/13735#comment:6
https://trac.sagemath.org/ticket/13735#comment:6
<p>
I started playing, but whenever I solved some problem, I introduced another. Since this is not really urgent (and not really necessary for the ticket I am actually working on), I leave it open for now, and we can decide later what we want to do with it.
</p>
<p>
Best, Christian
</p>
Ticketstumpc5Wed, 21 Nov 2012 18:41:27 GMTstatus changed
https://trac.sagemath.org/ticket/13735#comment:7
https://trac.sagemath.org/ticket/13735#comment:7
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
TicketnovoseltWed, 21 Nov 2012 19:16:51 GMT
https://trac.sagemath.org/ticket/13735#comment:8
https://trac.sagemath.org/ticket/13735#comment:8
<p>
My personal feeling is that latexing of expressions is disorganized in general and it is not clear how and where bugs have to be fixed, e.g. <a class="closed ticket" href="https://trac.sagemath.org/ticket/13356" title="defect: Wrong LaTeX for products of numbers (closed: fixed)">#13356</a> is much more serious than the issue here, but extra parentheses annoy me every once in while too, especially when with one ring all is OK and with another one not...
</p>
TicketvittucekMon, 08 Apr 2013 13:28:10 GMTattachment set
https://trac.sagemath.org/ticket/13735
https://trac.sagemath.org/ticket/13735
<ul>
<li><strong>attachment</strong>
set to <em>trac_13735_fix_latex_repr_lincomb.patch</em>
</li>
</ul>
<p>
fix the representation of linear combinations and include more testcases
</p>
TicketvittucekMon, 08 Apr 2013 13:31:00 GMT
https://trac.sagemath.org/ticket/13735#comment:9
https://trac.sagemath.org/ticket/13735#comment:9
<p>
Hi!
</p>
<p>
Scratching my own itch, I've changed the code for repr_lincomb to properly handle negative coefficients as well as zero. It required one change to how coeff_repr works which may cause problems. I've included testcases for repr_lincomb.
</p>
<p>
Is there a way how can I test the rest of sage only for this specific change? I don't want to run all the numerical test which take quite a while on my machine.
</p>
Ticketstumpc5Mon, 08 Apr 2013 15:46:58 GMT
https://trac.sagemath.org/ticket/13735#comment:10
https://trac.sagemath.org/ticket/13735#comment:10
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/13735#comment:9" title="Comment 9">vittucek</a>:
</p>
<blockquote class="citation">
<p>
Hi!
</p>
<p>
Scratching my own itch, I've changed the code for repr_lincomb to properly handle negative coefficients as well as zero. It required one change to how coeff_repr works which may cause problems. I've included testcases for repr_lincomb.
</p>
</blockquote>
<p>
feel free to take over this patch if you like - I stopped working on it since solving one issue somewhere caused others in other places.
</p>
<p>
To see if your patch causes problems somewhere, you cannot do much that running the complete doctests...
</p>
<p>
Cheers, Christian
</p>
TicketvittucekMon, 08 Apr 2013 17:20:59 GMT
https://trac.sagemath.org/ticket/13735#comment:11
https://trac.sagemath.org/ticket/13735#comment:11
<p>
For future reference, this is the list of modules that fail after my patch. Some of these failures are clearly bugs (e.g. /tmp/sage-5.8/devel/sage/sage/combinat/sf/hall_littlewood.py)
</p>
<p>
devel/sage/sage/structure/formal_sum.py
devel/sage/sage/rings/polynomial/plural.pyx
devel/sage/sage/modules/finite_submodule_iter.pyx
devel/sage/sage/combinat/free_module.py
devel/sage/sage/combinat/root_system/weyl_characters.py
devel/sage/sage/combinat/sf/llt.py
devel/sage/sage/combinat/sf/hall_littlewood.py
devel/sage/sage/combinat/sf/sfa.py
devel/sage/sage/combinat/sf/k_dual.py
devel/sage/sage/algebras/free_algebra_element.py
devel/sage/sage/algebras/group_algebra_new.py
devel/sage/sage/algebras/free_algebra_quotient_element.py
devel/sage/sage/algebras/iwahori_hecke_algebra.py
devel/sage/sage/algebras/steenrod/steenrod_algebra_mult.py
devel/sage/sage/algebras/steenrod/steenrod_algebra.py
devel/sage/sage/schemes/elliptic_curves/monsky_washnitzer.py
devel/sage/sage/schemes/generic/divisor.py
devel/sage/sage/modular/modsym/boundary.py
</p>
TicketnovoseltMon, 08 Apr 2013 20:25:13 GMT
https://trac.sagemath.org/ticket/13735#comment:12
https://trac.sagemath.org/ticket/13735#comment:12
<p>
Can we please have a space after the leading minus? Not <code>-x - x^2</code> but rather <code>- x - x^2</code> etc.
</p>
TicketvittucekMon, 08 Apr 2013 20:34:30 GMT
https://trac.sagemath.org/ticket/13735#comment:13
https://trac.sagemath.org/ticket/13735#comment:13
<p>
Sure. That's a trivial change.
</p>
TicketvittucekMon, 08 Apr 2013 20:54:30 GMT
https://trac.sagemath.org/ticket/13735#comment:14
https://trac.sagemath.org/ticket/13735#comment:14
<p>
I am in the middle of bugfixing right now.
</p>
<pre class="wiki">sage: R.<q> = ZZ[]
sage: A2 = WeylCharacterRing(['A',2], base_ring = R, style="coroots")
sage: [A2(x) for x in [-1]]
[-1*A2(0,0)]
</pre><p>
This is because the type of the coefficient -1 is sage.rings.polynomial.polynomial_integer_dense_flint.Polynomial_integer_dense_flint and the test c < 0 evaluates to False
</p>
<p>
Is this a bug or intended behaviour?
</p>
TicketvittucekMon, 08 Apr 2013 21:37:46 GMT
https://trac.sagemath.org/ticket/13735#comment:15
https://trac.sagemath.org/ticket/13735#comment:15
<p>
Similar issue arises in combinat/sf/llt.py, combinat/sf/k_dual.py and combinat/sf/hall_littlewood.py, where -t > 0 and type(-t) == sage.rings.fraction_field_element.FractionFieldElement_1poly_field
</p>
<p>
I think this should be a bug since c=-t leads to
</p>
<pre class="wiki">
sage: c > 0 and -c > 0
True
</pre>
TicketvittucekMon, 08 Apr 2013 22:57:35 GMTattachment set
https://trac.sagemath.org/ticket/13735
https://trac.sagemath.org/ticket/13735
<ul>
<li><strong>attachment</strong>
set to <em>failures.txt</em>
</li>
</ul>
TicketvittucekMon, 08 Apr 2013 23:01:54 GMT
https://trac.sagemath.org/ticket/13735#comment:16
https://trac.sagemath.org/ticket/13735#comment:16
<p>
I've hacked around the issues I mentioned and uploaded a bugfixed version of the patch. Right now I think that the only failures are due to the desired changes in output. Please see attached failures.txt and confirm this.
</p>
<p>
As for the space after leading minus as suggested by novoselt -- it can be done easily, but a lot of doctests expects it to be not there; i.e. it would require more "repairs" to doctests than current version. I think we need broader agreement to put that space there.
</p>
TicketvittucekWed, 15 May 2013 16:47:59 GMT
https://trac.sagemath.org/ticket/13735#comment:17
https://trac.sagemath.org/ticket/13735#comment:17
<p>
make ptestlong in Sage 5.9 produced only the following error:
</p>
<pre class="wiki">./sage sage -t --long devel/sage/sage/structure/sage_object.pyx
File "sage", line 31
resolvelinks() {
^
SyntaxError: invalid syntax
</pre><p>
Thus my patch is still valid.
</p>
TicketnovoseltWed, 15 May 2013 18:27:38 GMT
https://trac.sagemath.org/ticket/13735#comment:18
https://trac.sagemath.org/ticket/13735#comment:18
<p>
Is the last patch the only one that has to be applied? It should also list your full name in the header, and if this is ready for review - please add your name to the authors and switch to "needs review"!
</p>
TicketvittucekWed, 15 May 2013 18:51:16 GMTstatus, author changed
https://trac.sagemath.org/ticket/13735#comment:19
https://trac.sagemath.org/ticket/13735#comment:19
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>author</strong>
changed from <em>Christian Stump</em> to <em>Christian Stump, Vít Tuček</em>
</li>
</ul>
<p>
Yes. The last patch is the only one to be applied.
</p>
TicketnovoseltWed, 15 May 2013 18:53:10 GMTdescription changed
https://trac.sagemath.org/ticket/13735#comment:20
https://trac.sagemath.org/ticket/13735#comment:20
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/13735?action=diff&version=20">diff</a>)
</li>
</ul>
TicketnovoseltWed, 15 May 2013 20:50:06 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/13735#comment:21
https://trac.sagemath.org/ticket/13735#comment:21
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Andrey Novoseltsev</em>
</li>
</ul>
TicketjdemeyerThu, 16 May 2013 06:22:41 GMTstatus changed
https://trac.sagemath.org/ticket/13735#comment:22
https://trac.sagemath.org/ticket/13735#comment:22
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Could this have caused
</p>
<pre class="wiki">sage -t --long devel/sage/sage/modular/modsym/boundary.py
**********************************************************************
File "devel/sage/sage/modular/modsym/boundary.py", line 1299, in sage.modular.modsym.boundary.BoundarySpace_wtk_eps._coerce_cusp
Failed example:
[ B(Cusp(i,13)) for i in range(13) ]
Expected:
[[0],
[1/13],
(-zeta4)*[1/13],
[1/13],
(-1)*[1/13],
(-zeta4)*[1/13],
(-zeta4)*[1/13],
zeta4*[1/13],
zeta4*[1/13],
[1/13],
(-1)*[1/13],
zeta4*[1/13],
(-1)*[1/13]]
Got:
[[0], [1/13], -zeta4*[1/13], [1/13], -[1/13], -zeta4*[1/13], -zeta4*[1/13], zeta4*[1/13], zeta4*[1/13], [1/13], -[1/13], zeta4*[1/13], -[1/13]]
**********************************************************************
File "devel/sage/sage/modular/modsym/boundary.py", line 1323, in sage.modular.modsym.boundary.BoundarySpace_wtk_eps._coerce_cusp
Failed example:
[ B(Cusp(i,13)) for i in range(13) ]
Expected:
[0,
[1/13],
(-zeta4)*[1/13],
[1/13],
(-1)*[1/13],
(-zeta4)*[1/13],
(-zeta4)*[1/13],
zeta4*[1/13],
zeta4*[1/13],
[1/13],
(-1)*[1/13],
zeta4*[1/13],
(-1)*[1/13]]
Got:
[0, [1/13], -zeta4*[1/13], [1/13], -[1/13], -zeta4*[1/13], -zeta4*[1/13], zeta4*[1/13], zeta4*[1/13], [1/13], -[1/13], zeta4*[1/13], -[1/13]]
**********************************************************************
</pre><pre class="wiki">sage -t --long devel/sage/sage/modular/modsym/ambient.py
**********************************************************************
File "devel/sage/sage/modular/modsym/ambient.py", line 2100, in sage.modular.modsym.ambient.ModularSymbolsAmbient.twisted_winding_element
Failed example:
M.twisted_winding_element(0,eps)
Expected:
2*(1,23) + (-2)*(1,32) + 2*(1,34)
Got:
2*(1,23) - 2*(1,32) + 2*(1,34)
**********************************************************************
</pre><pre class="wiki">**********************************************************************
File "devel/sage/sage/tests/book_schilling_zabrocki_kschur_primer.py", line 571, in sage.tests.book_schilling_zabrocki_kschur_primer
Failed example:
p(ks3z[2, 2, 2, 2, 2, 2, 2, 2]) # long time (17s on sage.math, 2013)
Expected:
1/12*p[4, 4, 4, 4] + 1/4*p[8, 8] + (-1/3)*p[12, 4]
Got:
1/12*p[4, 4, 4, 4] + 1/4*p[8, 8] - 1/3*p[12, 4]
**********************************************************************
File "devel/sage/sage/tests/book_schilling_zabrocki_kschur_primer.py", line 573, in sage.tests.book_schilling_zabrocki_kschur_primer
Failed example:
p(ks3[2,2])
Expected:
1/12*p[1, 1, 1, 1] + 1/4*p[2, 2] + (-1/3)*p[3, 1]
Got:
1/12*p[1, 1, 1, 1] + 1/4*p[2, 2] - 1/3*p[3, 1]
**********************************************************************
File "devel/sage/sage/tests/book_schilling_zabrocki_kschur_primer.py", line 575, in sage.tests.book_schilling_zabrocki_kschur_primer
Failed example:
p(ks3[2,2]).plethysm(p[4])
Expected:
1/12*p[4, 4, 4, 4] + 1/4*p[8, 8] + (-1/3)*p[12, 4]
Got:
1/12*p[4, 4, 4, 4] + 1/4*p[8, 8] - 1/3*p[12, 4]
**********************************************************************
</pre>
TicketjdemeyerThu, 16 May 2013 06:29:47 GMTdependencies set
https://trac.sagemath.org/ticket/13735#comment:23
https://trac.sagemath.org/ticket/13735#comment:23
<ul>
<li><strong>dependencies</strong>
set to <em>#4327</em>
</li>
</ul>
<p>
With <a class="closed ticket" href="https://trac.sagemath.org/ticket/4327" title="enhancement: Refactor and extend root systems plots (closed: fixed)">#4327</a>:
</p>
<pre class="wiki">sage -t devel/sage/sage/combinat/root_system/root_lattice_realizations.py
**********************************************************************
File "devel/sage/sage/combinat/root_system/root_lattice_realizations.py", line 1840, in sage.combinat.root_system.root_lattice_realizations.RootLatticeRealizations.Pare
ntMethods.plot_roots
Failed example:
list(RootSystem(["A",2]).root_lattice().plot_roots("all"))
Expected:
[Arrow from (0.0,0.0) to (1.0,0.0),
Text '$\alpha_{1}$' at the point (1.05,0.0),
Arrow from (0.0,0.0) to (0.0,1.0),
Text '$\alpha_{2}$' at the point (0.0,1.05),
Arrow from (0.0,0.0) to (1.0,1.0),
Text '$\alpha_{1} + \alpha_{2}$' at the point (1.05,1.05),
Arrow from (0.0,0.0) to (-1.0,0.0),
Text '$\left(-1\right)\alpha_{1}$' at the point (-1.05,0.0),
Arrow from (0.0,0.0) to (0.0,-1.0),
Text '$\left(-1\right)\alpha_{2}$' at the point (0.0,-1.05),
Arrow from (0.0,0.0) to (-1.0,-1.0),
Text '$\left(-1\right)\alpha_{1} + \left(-1\right)\alpha_{2}$' at the point (-1.05,-1.05)]
Got:
[Arrow from (0.0,0.0) to (1.0,0.0), Text '$\alpha_{1}$' at the point (1.05,0.0), Arrow from (0.0,0.0) to (0.0,1.0), Text '$\alpha_{2}$' at the point (0.0,1.05), Arr
ow from (0.0,0.0) to (1.0,1.0), Text '$\alpha_{1} + \alpha_{2}$' at the point (1.05,1.05), Arrow from (0.0,0.0) to (-1.0,0.0), Text '$-\alpha_{1}$' at the point (-1.05,
0.0), Arrow from (0.0,0.0) to (0.0,-1.0), Text '$-\alpha_{2}$' at the point (0.0,-1.05), Arrow from (0.0,0.0) to (-1.0,-1.0), Text '$-\alpha_{1} - \alpha_{2}$' at the p
oint (-1.05,-1.05)]
**********************************************************************
</pre>
TicketvittucekThu, 16 May 2013 07:30:11 GMT
https://trac.sagemath.org/ticket/13735#comment:24
https://trac.sagemath.org/ticket/13735#comment:24
<p>
The only bug I see is the change from [0] to 0, otherwise it is the correct behaviour. If you disagree, then we need to figure out what exactly _is_ the expected behaviour of rep().
</p>
<p>
Regarding the first issue - my version of sage (5.9 + my patch) does not raise any errors. Where can I get the code to debug this?
</p>
TicketvittucekThu, 16 May 2013 07:33:19 GMT
https://trac.sagemath.org/ticket/13735#comment:25
https://trac.sagemath.org/ticket/13735#comment:25
<p>
Sorry, I mangled two testcases together. So it's again just the question of replacing (-1)* by unary minus and replacing (-x)* by -x* which I think is correct.
</p>
TicketjdemeyerThu, 16 May 2013 08:40:05 GMTdependencies changed
https://trac.sagemath.org/ticket/13735#comment:26
https://trac.sagemath.org/ticket/13735#comment:26
<ul>
<li><strong>dependencies</strong>
changed from <em>#4327</em> to <em>sage-5.10.beta4</em>
</li>
</ul>
<p>
You're right about tests passing on sage-5.9 (and even sage-5.10.beta3). I recommend you to wait for sage-5.10.beta4 and then rebase to that.
</p>
TicketvittucekThu, 16 May 2013 08:47:07 GMT
https://trac.sagemath.org/ticket/13735#comment:27
https://trac.sagemath.org/ticket/13735#comment:27
<p>
Can I somehow get notified when sage reaches beta4? This patch lies here for 5 weeks and could have made it into 5.9 if I knew that one has to change the status to needs_review. (Developer's guide is somehow lacking in this regard.)
</p>
TicketjdemeyerThu, 16 May 2013 08:51:48 GMT
https://trac.sagemath.org/ticket/13735#comment:28
https://trac.sagemath.org/ticket/13735#comment:28
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/13735#comment:27" title="Comment 27">vittucek</a>:
</p>
<blockquote class="citation">
<p>
Can I somehow get notified when sage reaches beta4?
</p>
</blockquote>
<p>
I will personally notify you.
</p>
TicketvittucekThu, 16 May 2013 08:56:55 GMT
https://trac.sagemath.org/ticket/13735#comment:29
https://trac.sagemath.org/ticket/13735#comment:29
<p>
Thanks. So just to be clear, the plan is to wait for 5.10 beta4 and then fix all the docstrings or possible bugs and turn the status of this patch to needs_review?
</p>
TicketjdemeyerThu, 16 May 2013 09:18:29 GMT
https://trac.sagemath.org/ticket/13735#comment:30
https://trac.sagemath.org/ticket/13735#comment:30
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/13735#comment:29" title="Comment 29">vittucek</a>:
</p>
<blockquote class="citation">
<p>
Thanks. So just to be clear, the plan is to wait for 5.10 beta4 and then fix all the docstrings or possible bugs and turn the status of this patch to needs_review?
</p>
</blockquote>
<p>
Yes, exactly.
</p>
TicketnthieryThu, 16 May 2013 13:10:41 GMT
https://trac.sagemath.org/ticket/13735#comment:31
https://trac.sagemath.org/ticket/13735#comment:31
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/13735#comment:23" title="Comment 23">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
With <a class="closed ticket" href="https://trac.sagemath.org/ticket/4327" title="enhancement: Refactor and extend root systems plots (closed: fixed)">#4327</a>:
</p>
<pre class="wiki"> ...
Text '$\left(-1\right)\alpha_{1} + \left(-1\right)\alpha_{2}$' at the point (-1.05,-1.05)]
Got:
Text '$-\alpha_{1} - \alpha_{2}$' at the point (-1.05,-1.05)]
</pre></blockquote>
<p>
Sorry for the little conflict, and thank you for improving the output
of root system plots :-)
</p>
TicketjdemeyerTue, 21 May 2013 06:55:46 GMT
https://trac.sagemath.org/ticket/13735#comment:32
https://trac.sagemath.org/ticket/13735#comment:32
<p>
sage-5.10.beta4 has been released, you should rebase this patch.
</p>
TicketvittucekTue, 21 May 2013 17:17:16 GMTattachment set
https://trac.sagemath.org/ticket/13735
https://trac.sagemath.org/ticket/13735
<ul>
<li><strong>attachment</strong>
set to <em>trac_13735_fix_repr_lincomb.patch</em>
</li>
</ul>
<p>
final version tested on top of 5.10-beta4
</p>
TicketvittucekTue, 21 May 2013 17:18:36 GMTstatus changed
https://trac.sagemath.org/ticket/13735#comment:33
https://trac.sagemath.org/ticket/13735#comment:33
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketvittucekWed, 22 May 2013 12:33:31 GMTauthor changed
https://trac.sagemath.org/ticket/13735#comment:34
https://trac.sagemath.org/ticket/13735#comment:34
<ul>
<li><strong>author</strong>
changed from <em>Christian Stump, Vít Tuček</em> to <em>Vít Tuček</em>
</li>
</ul>
TicketjdemeyerWed, 22 May 2013 13:59:36 GMT
https://trac.sagemath.org/ticket/13735#comment:35
https://trac.sagemath.org/ticket/13735#comment:35
<p>
Andrey, can you review this again please?
</p>
TicketnovoseltWed, 22 May 2013 15:36:30 GMTstatus changed
https://trac.sagemath.org/ticket/13735#comment:36
https://trac.sagemath.org/ticket/13735#comment:36
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
The patch does not apply for me on 5.10-beta4 because of this hunk:
</p>
<pre class="wiki">--- root_lattice_realizations.py
+++ root_lattice_realizations.py
@@ -1845,11 +1845,11 @@
Arrow from (0.0,0.0) to (1.0,1.0),
Text '$\alpha_{1} + \alpha_{2}$' at the point (1.05,1.05),
Arrow from (0.0,0.0) to (-1.0,0.0),
- Text '$\left(-1\right)\alpha_{1}$' at the point (-1.05,0.0),
+ Text '$-\alpha_{1}$' at the point (-1.05,0.0),
Arrow from (0.0,0.0) to (0.0,-1.0),
- Text '$\left(-1\right)\alpha_{2}$' at the point (0.0,-1.05),
+ Text '$-\alpha_{2}$' at the point (0.0,-1.05),
Arrow from (0.0,0.0) to (-1.0,-1.0),
- Text '$\left(-1\right)\alpha_{1} + \left(-1\right)\alpha_{2}$' at the point (-1.05,-1.05)]
+ Text '$-\alpha_{1} - \alpha_{2}$' at the point (-1.05,-1.05)]
"""
plot_options = self.plot_parse_options(**options)
root_lattice = self.root_system.root_lattice()
sage/combinat/root_system/root_lattice_realizations.py.rej (END)
</pre>
TicketvittucekWed, 22 May 2013 15:59:30 GMTstatus changed
https://trac.sagemath.org/ticket/13735#comment:37
https://trac.sagemath.org/ticket/13735#comment:37
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_info</em>
</li>
</ul>
<p>
I've just tried it on a fresh beta4 install and it applied cleanly.
</p>
TicketnovoseltWed, 22 May 2013 16:01:22 GMT
https://trac.sagemath.org/ticket/13735#comment:38
https://trac.sagemath.org/ticket/13735#comment:38
<p>
Did you download it from trac? Maybe something got wrong while you were uploading it here?
</p>
TicketvittucekWed, 22 May 2013 16:06:50 GMT
https://trac.sagemath.org/ticket/13735#comment:39
https://trac.sagemath.org/ticket/13735#comment:39
<p>
Yes, I downloaded it from trac.
</p>
TicketnovoseltWed, 22 May 2013 19:14:42 GMT
https://trac.sagemath.org/ticket/13735#comment:40
https://trac.sagemath.org/ticket/13735#comment:40
<p>
I am still getting an error applying on beta4 without anything else. Can it be some caching issue that I am not getting the newest version of the patch? Does the hunk above seem to be correct at least?
</p>
TicketvittucekWed, 22 May 2013 19:35:33 GMT
https://trac.sagemath.org/ticket/13735#comment:41
https://trac.sagemath.org/ticket/13735#comment:41
<p>
I have no idea what might be the issue here. I've checked <a class="ext-link" href="http://trac.sagemath.org/sage_trac/ticket/4327"><span class="icon"></span>http://trac.sagemath.org/sage_trac/ticket/4327</a> and the patch there seems to agree with mine. I've downloaded & built another beta4 and applied it directly (i.e. patch -p1 < trac_13735_fix_repr_lincomb.patch) and it went without any problems.
</p>
TicketnovoseltWed, 22 May 2013 20:20:11 GMTstatus changed
https://trac.sagemath.org/ticket/13735#comment:42
https://trac.sagemath.org/ticket/13735#comment:42
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
</ul>
<p>
OK, my bad, sorry - I've messed up with symbolic links while switching to beta4, everything applies fine now, running tests!
</p>
TicketnovoseltWed, 22 May 2013 20:40:35 GMTstatus changed
https://trac.sagemath.org/ticket/13735#comment:43
https://trac.sagemath.org/ticket/13735#comment:43
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
TicketjdemeyerSun, 26 May 2013 19:49:14 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/13735#comment:44
https://trac.sagemath.org/ticket/13735#comment:44
<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.10.beta5</em>
</li>
</ul>
Ticket