Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#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: sage-7.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:

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 5 years ago by moritz

  • Branch set to u/moritz/21928

comment:2 Changed 5 years ago by moritz

  • Commit set to 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 5 years ago by moritz

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

comment:4 Changed 5 years ago by moritz

  • Status changed from new to needs_review

comment:5 Changed 5 years ago by vdelecroix

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 git

  • Commit changed from 54eef889d5d6e33368b64b77ab34cc730a02968a to 335bf30fddda1e8f452171b5233bc56a2c93f261

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 5 years ago by git

  • Commit changed from 335bf30fddda1e8f452171b5233bc56a2c93f261 to a645dacbb7f3f20f6425fc310198c9c111450b9b

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

a645dacfixing old doctest and adding a new one

comment:8 Changed 5 years ago by moritz

  • 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 slabbe

  • 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:

  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 5 years ago by git

  • Commit changed from a645dacbb7f3f20f6425fc310198c9c111450b9b to 2765115750e7f236e1c923a66d8172ed4e71adbe

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

2765115allow nterms as argmument and more line breaks

comment:11 Changed 5 years ago by git

  • Commit changed from 2765115750e7f236e1c923a66d8172ed4e71adbe to d55256c8879ae3a80b685e74540e738360ed857d

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

d55256cremoved one line

comment:12 Changed 5 years ago by moritz

  • Status changed from needs_work to needs_review

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

Last edited 5 years ago by moritz (previous) (diff)

comment:13 Changed 5 years ago by slabbe

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 slabbe

  • Status changed from needs_review to positive_review

comment:15 Changed 5 years ago by slabbe

  • Authors set to Moritz Firsching
  • Reviewers set to Vincent Delecroix, Sébastien Labbé

comment:16 Changed 5 years ago by vbraun

  • Branch changed from u/moritz/21928 to d55256c8879ae3a80b685e74540e738360ed857d
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:17 Changed 4 years ago by chapoton

  • Commit d55256c8879ae3a80b685e74540e738360ed857d deleted

in the future, remember to use python3 syntax for print

Note: See TracTickets for help on using tickets.