#21928 closed defect (fixed)
produce nice latex code for latex(continued_fraction(pi)) and other continued fractions
Reported by:  slabbe  Owned by:  

Priority:  major  Milestone:  sage7.5 
Component:  number theory  Keywords:  days79 
Cc:  vdelecroix  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: 
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 5 years ago by
 Branch set to u/moritz/21928
comment:2 Changed 5 years ago by
 Commit set to 54eef889d5d6e33368b64b77ab34cc730a02968a
comment:3 Changed 5 years ago by
(I hope my automatic white space correction didn't screw up the asciiart to much..)
comment:4 Changed 5 years ago by
 Status changed from new to needs_review
comment:5 Changed 5 years ago by
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 5 years ago by
 Commit changed from 54eef889d5d6e33368b64b77ab34cc730a02968a to 335bf30fddda1e8f452171b5233bc56a2c93f261
comment:7 Changed 5 years ago by
 Commit changed from 335bf30fddda1e8f452171b5233bc56a2c93f261 to a645dacbb7f3f20f6425fc310198c9c111450b9b
Branch pushed to git repo; I updated commit sha1. New commits:
a645dac  fixing old doctest and adding a new one

comment:8 Changed 5 years ago by
 Summary changed from latex(continued_fraction(pi)) does not return nice latex code to produce nice latex code for latex(continued_fraction(pi)) and other continued fractions
comment:9 Changed 5 years ago by
 Status changed from needs_review to needs_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:
 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 methodstr
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 fromlatex
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 moventerms=10
to the argument which will allow somebody to see 13 terms (with the ...).
 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 5 years ago by
 Commit changed from a645dacbb7f3f20f6425fc310198c9c111450b9b to 2765115750e7f236e1c923a66d8172ed4e71adbe
Branch pushed to git repo; I updated commit sha1. New commits:
2765115  allow nterms as argmument and more line breaks

comment:11 Changed 5 years ago by
 Commit changed from 2765115750e7f236e1c923a66d8172ed4e71adbe to d55256c8879ae3a80b685e74540e738360ed857d
Branch pushed to git repo; I updated commit sha1. New commits:
d55256c  removed one line

comment:12 Changed 5 years ago by
 Status changed from needs_work to needs_review
I addressed both points mentioned by slabbe; thanks for reviewing!
comment:13 Changed 5 years ago by
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 5 years ago by
 Status changed from needs_review to positive_review
comment:15 Changed 5 years ago by
 Reviewers set to Vincent Delecroix, Sébastien Labbé
comment:16 Changed 5 years ago by
 Branch changed from u/moritz/21928 to d55256c8879ae3a80b685e74540e738360ed857d
 Resolution set to fixed
 Status changed from positive_review to closed
comment:17 Changed 5 years ago by
 Commit d55256c8879ae3a80b685e74540e738360ed857d deleted
in the future, remember to use python3 syntax for print
I added a
_latex_
function to theContinuedFraction_base
base class, that displays 10 terms by default. Notice that the subclassContinuedFraction_periodic
has already its own_latex_
, printing all the terms.New commits:
adding latex support for real continued fractions