Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#21928 closed defect (fixed)

produce nice latex code for latex(continued_fraction(pi)) and other continued fractions

Reported by: Sébastien Labbé Owned by:
Priority: major Milestone: sage-7.5
Component: number theory Keywords: days79
Cc: Vincent Delecroix Merged in:
Authors: Moritz Firsching Reviewers: Vincent Delecroix, Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: d55256c (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

sage: latex(continued_fraction(pi))
\text{\texttt{[3;{ }7,{ }15,{ }1,{ }292,{ }1,{ }1,{ }1,{ }2,{ }1,{ }3,{ }1,{ }14,{ }2,{ }1,{ }1,{ }2,{ }2,{ }2,{ }2,{ }...]}}

Change History (17)

comment:1 Changed 6 years ago by Moritz Firsching

Branch: u/moritz/21928

comment:2 Changed 6 years ago by Moritz Firsching

Commit: 54eef889d5d6e33368b64b77ab34cc730a02968a

I added a _latex_ function to the ContinuedFraction_base base class, that displays 10 terms by default. Notice that the subclass ContinuedFraction_periodic has already its own _latex_, printing all the terms.


New commits:

54eef88adding latex support for real continued fractions

comment:3 Changed 6 years ago by Moritz Firsching

(I hope my automatic white space correction didn't screw up the ascii-art to much..)

comment:4 Changed 6 years ago by Moritz Firsching

Status: newneeds_review

comment:5 Changed 6 years ago by Vincent Delecroix

Would be better to have only one place with _latex_. With your version we still have

sage: K.<sqrt2> = QuadraticField(2)
sage: continued_fraction(sqrt2)
[1; (2)*]
sage: latex(continued_fraction(sqrt2))
Traceback (most recent call last):
...
NotImplementedError: latex not implemented for non rational continued fractions

comment:6 Changed 6 years ago by git

Commit: 54eef889d5d6e33368b64b77ab34cc730a02968a335bf30fddda1e8f452171b5233bc56a2c93f261

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

0606ccdadded latex output to continued fraction of irrational algebraic numbers
335bf30Merge branch 'u/moritz/21928' of git://trac.sagemath.org/sage into u/moritz/ticket/21928

comment:7 Changed 6 years ago by git

Commit: 335bf30fddda1e8f452171b5233bc56a2c93f261a645dacbb7f3f20f6425fc310198c9c111450b9b

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

a645dacfixing old doctest and adding a new one

comment:8 Changed 6 years ago by Moritz Firsching

Summary: latex(continued_fraction(pi)) does not return nice latex codeproduce nice latex code for latex(continued_fraction(pi)) and other continued fractions

comment:9 Changed 6 years ago by Sébastien Labbé

Status: needs_reviewneeds_work

Thanks for proposing a fix to this issue. I just downloaded the branch. It works great.

Here are two relatively small comments that I would suggest to change before positive review:

  1. I think the nterms=10 would be better put as an argument with a default value of 10 for the _latex_ method like it is done for the method str of the same object. The first thing that will happen is that somebody want to see 13 terms (with the ...). I know that _latex_ method is used from latex function which might not forward arguments *args, **kwds. Something more complicated involving a new method could be done like here but I do not think it is necessary. To my opinion, I would just move nterms=10 to the argument which will allow somebody to see 13 terms (with the ...).
  1. The two output lines in doctests are very long. I suggest to wrap them to keep less than 80 characters on each line. The doctest framework won't notice the newlines. Alternatively, the code could include well placed \n so that \frac{\displaystyle are all well aligned vertically.

comment:10 Changed 6 years ago by git

Commit: a645dacbb7f3f20f6425fc310198c9c111450b9b2765115750e7f236e1c923a66d8172ed4e71adbe

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

2765115allow nterms as argmument and more line breaks

comment:11 Changed 6 years ago by git

Commit: 2765115750e7f236e1c923a66d8172ed4e71adbed55256c8879ae3a80b685e74540e738360ed857d

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

d55256cremoved one line

comment:12 Changed 6 years ago by Moritz Firsching

Status: needs_workneeds_review

I adressed both points mentioned by slabbe; thanks for reviewing!

Version 0, edited 6 years ago by Moritz Firsching (next)

comment:13 Changed 6 years ago by Sébastien Labbé

sage -grep continued_fraction lists 20 files and sage -tp --long on them gives all tests passed:

sage -tp --long src/sage/arith/misc.py src/sage/calculus/wester.py src/sage/combinat/words/word_generators.py src/sage/databases/oeis.py src/sage/geometry/cone.py src/sage/modular/modform_hecketriangle/hecke_triangle_group_element.py src/sage/modular/modform_hecketriangle/hecke_triangle_groups.py src/sage/modular/modform_hecketriangle/readme.py src/sage/modular/modsym/ambient.py src/sage/modular/modsym/modular_symbols.py src/sage/modular/pollack_stevens/manin_map.py src/sage/plot/misc.py src/sage/rings/all.py src/sage/rings/contfrac.py src/sage/rings/continued_fraction.py src/sage/rings/number_field/number_field_element_quadratic.pyx src/sage/rings/rational.pyx src/sage/rings/real_lazy.pyx src/sage/tests/book_stein_ent.py src/sage/tests/book_stein_modform.py

I am currently building the documentation to make sure...

comment:14 Changed 6 years ago by Sébastien Labbé

Status: needs_reviewpositive_review

comment:15 Changed 6 years ago by Sébastien Labbé

Authors: Moritz Firsching
Reviewers: Vincent Delecroix, Sébastien Labbé

comment:16 Changed 6 years ago by Volker Braun

Branch: u/moritz/21928d55256c8879ae3a80b685e74540e738360ed857d
Resolution: fixed
Status: positive_reviewclosed

comment:17 Changed 6 years ago by Frédéric Chapoton

Commit: d55256c8879ae3a80b685e74540e738360ed857d

in the future, remember to use python3 syntax for print

Note: See TracTickets for help on using tickets.