unnecessary blank in latex expression of multivariate polynomial

[dkrenn]

In the LaTeX representation of a multivariate polynomial is an unnecessary blank when variable is concatenated with a -, see

sage: R.<X,Y> = ZZ[]
sage: latex(X-Y)
X -  Y
sage: latex(X^2-X)
X^{2} -  X

It does not happen with +

sage: latex(X+Y)
X + Y

or with a constant

sage: latex(X-1) 
X - 1

In the univarate case the problem does not appear.

Note that this "wrong" output is no problem (i.e. a correct input for LaTeX)

[ppurka]

Does this really need a fix? The latex output can be used in two ways:

1. Input to a latex document.
2. Display on the notebook with typeset.

In both the cases, the output will be fine.

[aapitzsch]

The attached patch removes the unnecessary blank.

But now we also have

sage: latex(-X^2-X)
-X^{2} - X

instead of

sage: latex(-X^2-X)
- X^{2} - X

Is this reasonable or should we find a solution to keep the blank between the leading variable and its sign ("-")?

[novoselt]

No, it is not reasonable. As it was pointed out, the current output is correct and the only reason to "fix" is aesthetics. From this point of view, getting rid of a double space and swallowing a single one is not good - better to have more spaces then less.

[aapitzsch]

Currently we have

sage: latex(-x^2-x)
-x^{2} - x
sage: R.<y> = ZZ[]  
sage: latex(-y^2-y)
-y^{2} - y

Notice, there is no blank in the leading term, if it's no multivariate polynomial. So why do we need one if it's a multivariate polynomial?

IMO it should be consistent.

[novoselt]

Replying to aapitzsch:

IMO it should be consistent.

Yes, and this means that there is always exactly one space, if it is missing, it is the same "bug" as with double spaces or even worse since it makes reading more difficult and interferes with word wrapping.

A more useful LaTeX alteration than decreasing spaces would be getting rid of {} around singletons, i.e. instead of x_{1}^{2} it would be better to have x_1^2, again for reasons of readability and subsequent "manual manipulations". Macaulay2 is making such a change, as can be seen in one of the failing tests here: ​http://trac.sagemath.org/sage_trac/ticket/11710#comment:23

[aapitzsch]

Apply only trac_12908_latex_blank.patch.

This patch doesn't fix all "space bugs" in LaTeX expressions, only those which are related to the bug mentioned at the top.

[dsmatth]

I attempted to apply trac_12908_latex_blank.patch to Sage 5.10, but it appears rotted.

~/sage/devel/sage$ hg qimport trac_12908_latex_blank.patch                                                                                   
adding trac_12908_latex_blank.patch to series file                                                                                           
~/sage/devel/sage$ hg qpush                                                                                                                  
applying trac_12908_latex_blank.patch                                                                                                        
patching file doc/de/tutorial/latex.rst                                                                                                      
Hunk #1 succeeded at 73 with fuzz 1 (offset -5 lines).                                                                                       
patching file doc/en/tutorial/latex.rst                                                                                                      
Hunk #1 succeeded at 70 with fuzz 1 (offset -10 lines).                                                                                      
patching file sage/algebras/free_algebra_quotient_element.py                                                                                 
Hunk #1 FAILED at 136                                                                                                                        
1 out of 1 hunks FAILED -- saving rejects to file sage/algebras/free_algebra_quotient_element.py.rej                                         
patching file sage/combinat/free_module.py                                                                                                   
Hunk #1 FAILED at 1032                                                                                                                       
1 out of 1 hunks FAILED -- saving rejects to file sage/combinat/free_module.py.rej                                                           
patching file sage/rings/polynomial/polynomial_quotient_ring_element.py                                                                      
Hunk #1 FAILED at 185                                                                                                                        
1 out of 1 hunks FAILED -- saving rejects to file sage/rings/polynomial/polynomial_quotient_ring_element.py.rej                              
patch failed, unable to continue (try -v)                                                                                                    
patch failed, rejects left in working dir                                                                                                    
errors during apply, please fix and refresh trac_12908_latex_blank.patch 

On a side note, I would like to vote that the space between the sign and leading term should not be there. I am new to sage and trac, but I prefer

-X - Y

as opposed to

- X - Y

The leading negative indicates an inverse where as the subtraction between objects is an operation (adding an inverse). That is:

-X + -Y = -X - Y

Since the original patch has the same problems applying, I will change the status to needs work.

[novoselt]

Are there any particular style guidelines that recommend smaller space for unitary minus? I personally prefer the same space as for binary for consistency.

[rws]

FWIW, Don Knuth doesn't use spaces in his TeXbook. A better style guide you won't get.

[gh-durgeshra]

Testing:

sage: R.<X,Y> = ZZ[]
sage: latex(X-Y)
X - Y

(single space present between '-' and Y)

sage: latex(X^2-X)
X^{2} - X

(single space present between '-' and X)

---

New commits:

​7573eb9

Remove unnecessary blank in latex expression of multivariate polynomial

[embray]

Thanks! Please add the tests to the source in a follow-up commit. You can add the test cases in the docstring for singular_polynomial_latex. You can follow the format of the one existing regression test that's already there. Just put this one after the test for #11186. I would also add a test for the leading - case as in:

sage: latex(-x^2-x)
-x^{2} - x
sage: R.<y> = ZZ[]  
sage: latex(-y^2-y)
-y^{2} - y

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​ea2d10f

Add test case

[gh-durgeshra]

Replying to embray:

Thanks! Please add the tests to the source in a follow-up commit. You can add the test cases in the docstring for singular_polynomial_latex. You can follow the format of the one existing regression test that's already there. Just put this one after the test for #11186. I would also add a test for the leading - case as in:

sage: latex(-x^2-x)
-x^{2} - x
sage: R.<y> = ZZ[]  
sage: latex(-y^2-y)
-y^{2} - y

Done.

[charpent]

Dear Durgesh,

1) Welcome aboard !

2) About your proposal :

Replying to gh-durgeshra:

[ Snip... ]

Done.

A better way to pass that information would have been to change the state of the ticket to needs_work : this way, all people working (or having worked) on that ticket would have received the (useful) info that a proposed fix needed review.

I'll try to have a look later this week-end (possibly today).

[embray]

src/sage/libs/singular/polynomial.pyx

@@ -460 +460 @@
         sage: P.<v,w> = K[]
         sage: latex((z+1)*v*w - z*w^2 + z*v + z^2*w + z + 1)
         \left(z + 1\right) v w - z w^{2} + z v + \left(-z - 1\right) w + z + 1
+
+    Demonstrate that there is no blank in latex expression of multivariate
+    polynomial (:trac:`12908`)::
+        sage: R.<X,Y> = ZZ[]
+        sage: latex(X-Y)
+        X - Y
+        sage: latex(X^2-X)
+        X^{2} - X
     """
     poly = ""
     cdef unsigned long e

Please include a blank line after the :: as in other examples. I think it isn't strictly needed but it's stylistically more consistent. I would also reword the text to "Demonstrate that there are no extra blanks in latex expressions of multivariate polynomial". The wording you have is confusing: There are blanks, and this demonstrates that there are. What we want is not extra, superfluous blanks. Please also add an example with a leading unary - as mentioned in comment:18.

Sorry for the nitpicks but that's why it's nice for new contributors to start with an easy ticket :) Looks like you're almost there.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​75d9956

Nit fixes

[charpent]

Replying to embray:

[ Snip... ]

Sorry for the nitpicks but that's why it's nice for new contributors to start with an easy ticket :) Looks like you're almost there.

Ahem...

Apart from (justifiable) nitpickings, this (correct) fix breaks three other doctests that rely on a specific latex representation :

----------------------------------------------------------------------
sage -t --long --warn-long 144.0 src/sage/modular/modform_hecketriangle/graded_ring_element.py  # 1 doctest failed
sage -t --long --warn-long 144.0 src/sage/modular/modform_hecketriangle/element.py  # 1 doctest failed
sage -t --long --warn-long 144.0 src/sage/algebras/weyl_algebra.py  # 1 doctest failed
----------------------------------------------------------------------

Here are the errors :

charpent@zen-book-flip:/usr/local/sage-8$ sage -t --long --warn-long 144.0 src/sage/modular/modform_hecketriangle/graded_ring_element.py
Running doctests with ID 2019-01-04-13-39-47-4a9651a6.
Git branch: t/12908/unnecessary_blank_in_latex_expression_of_multivariate_polynomial
Using --optional=dochtml,fricas,gap_packages,giacpy_sage,gmpy2,memlimit,mpir,python2,sage
Doctesting 1 file.
sage -t --long --warn-long 144.0 src/sage/modular/modform_hecketriangle/graded_ring_element.py
**********************************************************************
File "src/sage/modular/modform_hecketriangle/graded_ring_element.py", line 252, in sage.modular.modform_hecketriangle.graded_ring_element.?._latex_
Failed example:
    latex(QuasiModularFormsRing(n=infinity)(x*(x-y^2)*z))
Expected:
    - E_{4} f_{i}^{2} E_{2} + E_{4}^{2} E_{2}
Got:
    -E_{4} f_{i}^{2} E_{2} + E_{4}^{2} E_{2}
**********************************************************************
1 item had failures:
   1 of   7 in sage.modular.modform_hecketriangle.graded_ring_element.?._latex_
    [672 tests, 1 failure, 17.26 s]
----------------------------------------------------------------------
sage -t --long --warn-long 144.0 src/sage/modular/modform_hecketriangle/graded_ring_element.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 17.4 seconds
    cpu time: 17.0 seconds
    cumulative wall time: 17.3 seconds
charpent@zen-book-flip:/usr/local/sage-8$ sage -t --long --warn-long 144.0 src/sage/modular/modform_hecketriangle/element.py 
Running doctests with ID 2019-01-04-13-40-22-f8799c62.
Git branch: t/12908/unnecessary_blank_in_latex_expression_of_multivariate_polynomial
Using --optional=dochtml,fricas,gap_packages,giacpy_sage,gmpy2,memlimit,mpir,python2,sage
Doctesting 1 file.
sage -t --long --warn-long 144.0 src/sage/modular/modform_hecketriangle/element.py
**********************************************************************
File "src/sage/modular/modform_hecketriangle/element.py", line 111, in sage.modular.modform_hecketriangle.element.FormsElement._latex_
Failed example:
    latex(QuasiModularForms(n=infinity, k=8, ep=1)(x*(x-y^2)))
Expected:
    - E_{4} f_{i}^{2} + E_{4}^{2}
Got:
    -E_{4} f_{i}^{2} + E_{4}^{2}
**********************************************************************
1 item had failures:
   1 of   5 in sage.modular.modform_hecketriangle.element.FormsElement._latex_
    [83 tests, 1 failure, 2.43 s]
----------------------------------------------------------------------
sage -t --long --warn-long 144.0 src/sage/modular/modform_hecketriangle/element.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 7.5 seconds
    cpu time: 2.2 seconds
    cumulative wall time: 2.4 seconds
charpent@zen-book-flip:/usr/local/sage-8$ sage -t --long --warn-long 144.0 src/sage/algebras/weyl_algebra.py  # 1 doctest failed
Running doctests with ID 2019-01-04-13-40-51-fc1e6dd9.
Git branch: t/12908/unnecessary_blank_in_latex_expression_of_multivariate_polynomial
Using --optional=dochtml,fricas,gap_packages,giacpy_sage,gmpy2,memlimit,mpir,python2,sage
Doctesting 1 file.
sage -t --long --warn-long 144.0 src/sage/algebras/weyl_algebra.py
**********************************************************************
File "src/sage/algebras/weyl_algebra.py", line 112, in sage.algebras.weyl_algebra.repr_from_monomials
Failed example:
    latex(c*(a*a + 2)*b)
Expected:
    \left( - x - 2 \right)  e_{1} e_{2} - 4 x - 8
Got:
    \left( -x - 2 \right)  e_{1} e_{2} - 4 x - 8
**********************************************************************
1 item had failures:
   1 of  28 in sage.algebras.weyl_algebra.repr_from_monomials
    [159 tests, 1 failure, 0.18 s]
----------------------------------------------------------------------
sage -t --long --warn-long 144.0 src/sage/algebras/weyl_algebra.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 0.2 seconds
    cpu time: 0.2 seconds
    cumulative wall time: 0.2 seconds

In all cases, the change introduced by the ticket gives a correct LaTeX representation, differing just by the amount of space. These doctests should be fixed.

---

New commits:

​75d9956

Nit fixes

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​3ab45d5

Fix doctests

[charpent]

Merged on 8.6.rc0, passes ptestlong ==> positive_review.

[embray]

I figured there would be some new test failures as a result of this, and was going to check that next after all other details were dealt with? Also, I'd prefer people squash their commits when making lots of little minor changes so I will do that.

[embray]

Squashed. Thanks Durgesh.

---

New commits:

​86d9920

Remove unnecessary blank in latex expression of multivariate polynomial

[slelievre]

Have all authors got their names in the "Authors" field?

[gh-durgeshra]

I haven't. Will do right now. Edit: Done Replying to slelievre:

Have all authors got their names in the "Authors" field?

[slelievre]

I mean on the Trac ticket. Click "Modify Ticket", then add your full name to the Authors field.

[jhpalmieri]

The ticket's already been merged, so I don't know if the change in authors will have any effect.

[slelievre]

It will matter when producing the changelog for SageMath 8.7.

[jhpalmieri]

Oh, the changelog isn't just based on the git log?

[jdemeyer]

Replying to jhpalmieri:

Oh, the changelog isn't just based on the git log?

No, it's based on the information on Trac. In fact, the changelog-generating script predates the switch to git!