Opened 20 months ago

Closed 19 months ago

#31147 closed defect (fixed)

Add missing curly braces in latex of Kodaira symbol

Reported by: roed Owned by:
Priority: minor Milestone: sage-9.3
Component: elliptic curves Keywords:
Cc: Merged in:
Authors: David Roe Reviewers: Frédéric Chapoton, John Cremona, Samuel Lelièvre
Report Upstream: N/A Work issues:
Branch: ba6e4db (Commits, GitHub, GitLab) Commit: ba6e4db8870dd7dc59e22ef7a72904e42d158b03
Dependencies: Stopgaps:

Status badges

Description 

sage: latex(EllipticCurve([0, -1, 0, -101781377, -517919745247]).kodaira_symbol(2))                                                                                                                                               
I_64^{*}

It should be I_{64}^{*}.

Change History (18)

comment:1 Changed 20 months ago by roed

  • Branch set to u/roed/kod

comment:2 Changed 20 months ago by roed

  • Commit set to c487ec9987d75d1a12534cbd572eec88caf8f279
  • Status changed from new to needs_review

New commits:

​c487ec9Fix bug in latex for Kodaira symbols

comment:3 Changed 20 months ago by chapoton

the new doctest is failing..

Last edited 20 months ago by chapoton (previous) (diff)

comment:4 Changed 20 months ago by git

  • Commit changed from c487ec9987d75d1a12534cbd572eec88caf8f279 to 0d32acf216a89e1d7cf55164f2dbe04e7ae8525b

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

​0d32acfFix test output

comment:5 Changed 20 months ago by roed

Oops. Should be fixed now.

comment:6 Changed 20 months ago by slelievre

How about an f-string: 

-                self._latex = 'I_{' + nu.str() + '}^{*}'
+                self._latex = f'I_{{{nu}}}^{{*}}'

comment:7 Changed 20 months ago by chapoton

and the trac role syntax is plain wrong

comment:8 Changed 20 months ago by git

  • Commit changed from 0d32acf216a89e1d7cf55164f2dbe04e7ae8525b to 6b59bb0d984737156e92837611e628c742021df6

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

​6b59bb0Change formatting code, use correct trac role

comment:9 Changed 20 months ago by roed

I don't know why, but I often get the trac role syntax wrong, and forgot to check myself this time. I've fixed it.

I find an f-string less readable with so many curly braces floating around, but I've switched to using the % operator rather than adding strings.

comment:10 Changed 20 months ago by chapoton

the branch contains spurious changes in ring_extension

comment:11 Changed 19 months ago by cremona

I am happy with the changes to kodaira_symbol.py but think that the other changed file should not be in the commit.

comment:12 Changed 19 months ago by slelievre

  • Reviewers set to Frédéric Chapoton, John Cremona, Samuel Lelièvre

Okay with %s to avoid having to escape braces in an f-string.

comment:13 Changed 19 months ago by chapoton

  • Status changed from needs_review to needs_work

Because of the wrongly modified file, this currently needs work.

comment:14 Changed 19 months ago by chapoton

  • Branch changed from u/roed/kod to public/ticket/31147
  • Commit changed from 6b59bb0d984737156e92837611e628c742021df6 to ba6e4db8870dd7dc59e22ef7a72904e42d158b03

ok, I made a new branch myself.

New commits:

​ba6e4dbFix bug in latex for Kodaira symbols

comment:15 Changed 19 months ago by chapoton

  • Status changed from needs_work to needs_review

comment:16 Changed 19 months ago by chapoton

  • Status changed from needs_review to positive_review

I will allow myself to set to positive.

comment:17 Changed 19 months ago by roed

Sorry about the extra file, and thanks for fixing it!

comment:18 Changed 19 months ago by vbraun

  • Branch changed from public/ticket/31147 to ba6e4db8870dd7dc59e22ef7a72904e42d158b03
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.