Opened 7 years ago

Closed 4 weeks ago

# unnecessary blank in latex expression of multivariate polynomial

Reported by: Owned by: dkrenn was trivial sage-8.7 user interface beginner, latex, blank, multivariate polynomial charpent André Apitzsch, Durgesh Agrawal Debbie Matthews, Erik Bray, Emmanuel Charpentier N/A add additional regression test, formatting nitpicks 86d9920 (Commits)

### Description

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)

### comment:1 Changed 7 years ago by 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.

### comment:2 Changed 7 years ago by aapitzsch

• Authors set to André Apitzsch
• Status changed from new to needs_review

The attached patch removes the unnecessary blank.

But now we also have

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


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 ("-")?

### comment:3 Changed 7 years ago by novoselt

• Status changed from needs_review to needs_work

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.

### comment:4 follow-up: ↓ 5 Changed 7 years ago by 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.

### comment:5 in reply to: ↑ 4 Changed 7 years ago by novoselt

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

### comment:6 Changed 7 years ago by aapitzsch

• Status changed from needs_work to needs_review

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.

### comment:7 Changed 6 years ago by 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.

### comment:8 Changed 6 years ago by dsmatth

• Reviewers set to Debbie Matthews
• Status changed from needs_review to needs_work

### comment:9 Changed 6 years ago by 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.

### comment:10 Changed 6 years ago by jdemeyer

• Milestone changed from sage-5.11 to sage-5.12

### comment:11 Changed 5 years ago by vbraun_spam

• Milestone changed from sage-6.1 to sage-6.2

### comment:12 Changed 5 years ago by rws

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

### comment:13 Changed 5 years ago by vbraun_spam

• Milestone changed from sage-6.2 to sage-6.3

### comment:14 Changed 5 years ago by vbraun_spam

• Milestone changed from sage-6.3 to sage-6.4

### comment:15 Changed 7 weeks ago by embray

• Milestone sage-6.4 deleted

### comment:16 Changed 7 weeks ago by gh-durgeshra

• Branch set to u/gh-durgeshra/unnecessary_blank_in_latex_expression_of_multivariate_polynomial

### comment:17 Changed 7 weeks ago by 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

### comment:18 follow-up: ↓ 20 Changed 7 weeks ago by embray

• Reviewers changed from Debbie Matthews to Debbie Matthews, Erik Bray
• Work issues set to add regression tests as doctests to singular_polynomial_latex

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


### comment:19 Changed 7 weeks ago by git

• Commit changed from 7573eb9aecb377d47d199654b6ad591db13204cd to ea2d10f5c0083d54d2fd7b95da6d159199fd9df5

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

 ​ea2d10f Add test case

### comment:20 in reply to: ↑ 18 ; follow-up: ↓ 21 Changed 7 weeks ago by gh-durgeshra

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.

### comment:21 in reply to: ↑ 20 Changed 7 weeks ago by charpent

Dear Durgesh,

1) Welcome aboard !

[ 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).

### comment:22 Changed 7 weeks ago by gh-durgeshra

• Status changed from needs_work to needs_review

### comment:23 follow-up: ↓ 26 Changed 7 weeks ago by embray

• Status changed from needs_review to needs_work
• Work issues changed from add regression tests as doctests to singular_polynomial_latex to add additional regression test, formatting nitpicks
• ## src/sage/libs/singular/polynomial.pyx

diff --git a/src/sage/libs/singular/polynomial.pyx b/src/sage/libs/singular/polynomial.pyx
index 2e4efb1..a494142 100644
 a cdef object singular_polynomial_latex(poly *p, ring *r, object base, object late sage: P. = 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. = 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.

### comment:24 Changed 7 weeks ago by git

• Commit changed from ea2d10f5c0083d54d2fd7b95da6d159199fd9df5 to 75d995617ab858f83ee7e80e28c7de524770390a

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

 ​75d9956 Nit fixes

### comment:25 Changed 7 weeks ago by gh-durgeshra

• Status changed from needs_work to needs_review

### comment:26 in reply to: ↑ 23 Changed 7 weeks ago by charpent

[ 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 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 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

### comment:27 Changed 7 weeks ago by charpent

• Status changed from needs_review to needs_work

### comment:29 Changed 7 weeks ago by git

• Commit changed from 75d995617ab858f83ee7e80e28c7de524770390a to 3ab45d5fe2e106436b12e0c7cfed99d0416525fa

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

 ​3ab45d5 Fix doctests

### comment:30 Changed 7 weeks ago by gh-durgeshra

• Status changed from needs_work to needs_review

### comment:31 Changed 7 weeks ago by charpent

• Reviewers changed from Debbie Matthews, Erik Bray to Debbie Matthews, Erik Bray, Emmanuel Charpentier
• Status changed from needs_review to positive_review

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

### comment:32 Changed 7 weeks ago by 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.

### comment:33 Changed 7 weeks ago by embray

• Branch changed from u/gh-durgeshra/unnecessary_blank_in_latex_expression_of_multivariate_polynomial to public/ticket-12908
• Commit changed from 3ab45d5fe2e106436b12e0c7cfed99d0416525fa to 86d9920a0dea3d92b062f34636465991df0591a3

Squashed. Thanks Durgesh.

New commits:

 ​86d9920 Remove unnecessary blank in latex expression of multivariate polynomial

### comment:34 Changed 7 weeks ago by chapoton

• Milestone set to sage-8.7

### comment:35 Changed 4 weeks ago by vbraun

• Branch changed from public/ticket-12908 to 86d9920a0dea3d92b062f34636465991df0591a3
• Resolution set to fixed
• Status changed from positive_review to closed

### comment:36 follow-up: ↓ 37 Changed 4 weeks ago by slelievre

• Commit 86d9920a0dea3d92b062f34636465991df0591a3 deleted

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

### comment:37 in reply to: ↑ 36 Changed 4 weeks ago by gh-durgeshra

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

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

Last edited 4 weeks ago by gh-durgeshra (previous) (diff)

### comment:38 Changed 4 weeks ago by slelievre

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

### comment:39 Changed 4 weeks ago by gh-durgeshra

• Authors changed from André Apitzsch to André Apitzsch, Durgesh Agrawal

### comment:40 Changed 4 weeks ago by jhpalmieri

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

### comment:41 Changed 4 weeks ago by slelievre

It will matter when producing the changelog for SageMath 8.7.

### comment:42 follow-up: ↓ 43 Changed 4 weeks ago by jhpalmieri

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