Opened 21 months ago
Closed 17 months ago
#31586 closed defect (fixed)
Add _latex_ method to curves and improve formatting in schemes folder
Reported by:  Kwankyu Lee  Owned by:  

Priority:  major  Milestone:  sage9.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: 
Description (last modified by )
This ticket does
 add
_latex_
method to algebraic curves
 improve latex strings formatting in schemes folder
Change History (21)
comment:1 Changed 21 months ago by
Branch:  → u/klee/31586 

comment:2 Changed 21 months ago by
Commit:  → 752b0e42182d9a19014b87032ad5a96ae0e415a5 

comment:3 Changed 21 months ago by
Description:  modified (diff) 

Status:  new → needs_review 
Summary:  Fix a few typographic errors in latex strings in schemes folder → Add _latex_ to curves and fix a few typographic errors in schemes folder 
comment:4 Changed 21 months ago by
Authors:  → Kwankyu Lee 

comment:5 Changed 19 months ago by
Commit:  752b0e42182d9a19014b87032ad5a96ae0e415a5 → 56014e9a82aab82b9fc043bd2cd9c747d0a4091a 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
56014e9  Fix a few typographic errors in schemes folder

comment:6 Changed 19 months ago by
Description:  modified (diff) 

Summary:  Add _latex_ to curves and fix a few typographic errors in schemes folder → Add `_latex_` method to curves and fix a few typographic errors in schemes folder 
comment:7 Changed 19 months ago by
Summary:  Add `_latex_` method to curves and fix a few typographic errors in schemes folder → Add _latex_ method to curves and fix a few typographic errors in schemes folder 

comment:8 followup: 10 Changed 19 months ago by
Description:  modified (diff) 

Reviewers:  → Samuel Lelièvre 
Summary:  Add _latex_ method to curves and fix a few typographic errors in schemes folder → Add _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
Commit:  56014e9a82aab82b9fc043bd2cd9c747d0a4091a → 8f8b0e1ac63f0bd27689222484a9b14d96b156d2 

Branch pushed to git repo; I updated commit sha1. New commits:
8f8b0e1  Fixes by reviewer comments

comment:10 Changed 19 months ago by
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
Commit:  8f8b0e1ac63f0bd27689222484a9b14d96b156d2 → b5c6e5aff461f064c6f46a87e970e2be9727446d 

Branch pushed to git repo; I updated commit sha1. New commits:
b5c6e5a  Make pycodestyle plugin happy

comment:13 followup: 15 Changed 19 months ago by
Though I'm thinking this return 0
should be return QQ.zero()
...
comment:14 Changed 19 months ago by
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 Changed 19 months ago by
Replying to slelievre:
Though I'm thinking this
return 0
should bereturn 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:17 Changed 17 months ago by
Commit:  b5c6e5aff461f064c6f46a87e970e2be9727446d → 2a6dd749b21ae9a5cb326c79e7e2a4aa77da6965 

Status:  positive_review → needs_review 
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:
2a6dd74  Fix a few typographic errors in schemes folder

comment:18 Changed 17 months ago by
Status:  needs_review → positive_review 

comment:20 Changed 17 months ago by
Priority:  minor → major 

comment:21 Changed 17 months ago by
Branch:  u/klee/31586 → 2a6dd749b21ae9a5cb326c79e7e2a4aa77da6965 

Resolution:  → fixed 
Status:  positive_review → closed 
Branch pushed to git repo; I updated commit sha1. New commits:
Fix a few typographic errors in schemes folder