#31586 closed defect (fixed)

Add _latex_ method to curves and improve formatting in schemes folder

Reported by: Kwankyu Lee Owned by:
Priority: major Milestone: sage-9.4
Component: documentation Keywords:
Cc: Merged in:
Authors: Kwankyu Lee Reviewers: Samuel Lelièvre
Report Upstream: N/A Work issues:
Branch: 2a6dd74 (Commits, GitHub, GitLab) Commit: 2a6dd749b21ae9a5cb326c79e7e2a4aa77da6965
Dependencies: Stopgaps:

Status badges

Description (last modified by Samuel Lelièvre)

This ticket does

  1. add _latex_ method to algebraic curves
  1. improve latex strings formatting in schemes folder

Change History (21)

comment:1 Changed 21 months ago by Kwankyu Lee

Branch: u/klee/31586

comment:2 Changed 21 months ago by git

Commit: 752b0e42182d9a19014b87032ad5a96ae0e415a5

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

752b0e4Fix a few typographic errors in schemes folder

comment:3 Changed 21 months ago by Kwankyu Lee

Description: modified (diff)
Status: newneeds_review
Summary: Fix a few typographic errors in latex strings in schemes folderAdd _latex_ to curves and fix a few typographic errors in schemes folder

comment:4 Changed 21 months ago by Kwankyu Lee

Authors: Kwankyu Lee

comment:5 Changed 19 months ago by git

Commit: 752b0e42182d9a19014b87032ad5a96ae0e415a556014e9a82aab82b9fc043bd2cd9c747d0a4091a

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

56014e9Fix a few typographic errors in schemes folder

comment:6 Changed 19 months ago by Kwankyu Lee

Description: modified (diff)
Summary: Add _latex_ to curves and fix a few typographic errors in schemes folderAdd `_latex_` method to curves and fix a few typographic errors in schemes folder

comment:7 Changed 19 months ago by Kwankyu Lee

Summary: Add `_latex_` method to curves and fix a few typographic errors in schemes folderAdd _latex_ method to curves and fix a few typographic errors in schemes folder

comment:8 Changed 19 months ago by Samuel Lelièvre

Description: modified (diff)
Reviewers: Samuel Lelièvre
Summary: Add _latex_ method to curves and fix a few typographic errors in schemes folderAdd _latex_ method to curves and improve formatting in schemes folder

I would suggest adding line breaks and using formatted strings and raw strings as follows.

-            \text{Projective Plane curve over $\Bold{Q}$ defined by $-x^{3} + y^{2} z - 17 x z^{2} + y z^{2}$}
+            \text{Projective Plane curve over $\Bold{Q}$
+            defined by $-x^{3} + y^{2} z - 17 x z^{2} + y z^{2}$}
-            \text{Affine Plane curve over $\Bold{Q}$ defined by $-x^{3} + y^{2} - 17 x + y$}
+            \text{Affine Plane curve over $\Bold{Q}$
+            defined by $-x^{3} + y^{2} - 17 x + y$}
-        if self.defining_ideal().is_zero() and self.ambient_space().dimension() == 1:
-            return r"\text{{{} line over ${}$}}".format(self._repr_type(), latex(self.base_ring()))
-        else:
-            return r"\text{{{} curve over ${}$ defined by {}}}".format(self._repr_type(), latex(self.base_ring()),
-                 ', '.join(['${0}$'.format(latex(x)) for x in self.defining_polynomials()]))
+        if (self.defining_ideal().is_zero()
+                and self.ambient_space().dimension() == 1):
+            repr_type, ring = self._repr_type(), latex(self.base_ring())
+            return fr"\text{{{repr_type} line over ${ring}$}}"
+        else:
+            repr_type, ring = self._repr_type(), latex(self.base_ring())
+            pp = ', '.join(f'${latex(p)}$' for p in self.defining_polynomials())
+            return fr"\text{{{repr_type} curve over ${ring}$ defined by {pp}}}"
-        return 'Rational cohomology ring of a ' + self._variety._repr_()
+        return f'Rational cohomology ring of a {self._variety._repr_()}'
-        return 'H^\\ast\\left(' + self._variety._latex_() + ',' + latex(QQ) + '\\right)'
+        return fr'H^\ast\left({self._variety._latex_()},{latex(QQ)}\right)'

Other than that (but feel free to discard any or all of these suggestions), positive review if bots agree.

comment:9 Changed 19 months ago by git

Commit: 56014e9a82aab82b9fc043bd2cd9c747d0a4091a8f8b0e1ac63f0bd27689222484a9b14d96b156d2

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

8f8b0e1Fixes by reviewer comments

comment:10 in reply to:  8 Changed 19 months ago by Kwankyu Lee

Replying to slelievre:

Other than that (but feel free to discard any or all of these suggestions), positive review if bots agree.

Good suggestions. Thank you.

comment:11 Changed 19 months ago by git

Commit: 8f8b0e1ac63f0bd27689222484a9b14d96b156d2b5c6e5aff461f064c6f46a87e970e2be9727446d

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

b5c6e5aMake pycodestyle plugin happy

comment:12 Changed 19 months ago by Samuel Lelièvre

Status: needs_reviewpositive_review

Wonderful.

comment:13 Changed 19 months ago by Samuel Lelièvre

Though I'm thinking this return 0 should be return QQ.zero()...

Version 0, edited 19 months ago by Samuel Lelièvre (next)

comment:14 Changed 19 months ago by Samuel Lelièvre

Another way of putting it would be

-        if top_form.is_zero():
-            return 0
-        return top_form.lc() / self.volume_class().lc()
+        return (QQ.zero() if top_form.is_zero()
+                else top_form.lc() / self.volume_class().lc())

comment:15 in reply to:  13 Changed 19 months ago by Kwankyu Lee

Replying to slelievre:

Though I'm thinking this return 0 should be return QQ.zero()...

Perhaps it would be better to return the zero element of the base ring of the cohomology ring. However, it seems a pervasive convention in Sage just to return 0 regardless of the proper parent of the zero.

Anyway, it is not in the scope of this ticket to "fix" it. I just leave it as it is.

Thank you for the review.

comment:16 Changed 19 months ago by Samuel Lelièvre

I agree it is beyond the scope of this ticket.

comment:17 Changed 17 months ago by git

Commit: b5c6e5aff461f064c6f46a87e970e2be9727446d2a6dd749b21ae9a5cb326c79e7e2a4aa77da6965
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

2a6dd74Fix a few typographic errors in schemes folder

comment:18 Changed 17 months ago by Kwankyu Lee

Status: needs_reviewpositive_review

comment:19 Changed 17 months ago by Kwankyu Lee

Rebased on the latest beta

comment:20 Changed 17 months ago by Kwankyu Lee

Priority: minormajor

comment:21 Changed 17 months ago by Volker Braun

Branch: u/klee/315862a6dd749b21ae9a5cb326c79e7e2a4aa77da6965
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.