Opened 8 years ago

Closed 8 years ago

#17745 closed defect (fixed)

typo causes latex error in indexed generators

Reported by: Karl-Dieter Crisman Owned by:
Priority: minor Milestone: sage-6.5
Component: documentation Keywords:
Cc: Travis Scrimshaw, Nicolas M. Thiéry Merged in:
Authors: John Palmieri Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 0065db9 (Commits, GitHub, GitLab) Commit: 0065db912af0bebb7f1149089e3a1b5b7159cd97
Dependencies: Stopgaps:

Status badges

Description

See http://ask.sagemath.org/question/25763/incorrect-parsing-of-docstring-of-sagestructureindexed_generatorsindexedgenerators/

where a \left is displayed as <=ft. It seems that \le in \left gets replaced by <=.

This is from #15289.

Change History (26)

comment:1 Changed 8 years ago by Travis Scrimshaw

I believe this is a general issue with the parsing of latex docstrings to a more ascii version just being too greedy (because I saw it somewhere else IIRC). Do you happen to know where the code that does this substitution is?

comment:2 Changed 8 years ago by aapitzsch

Problem seems to be in src/sage/misc/sagedoc.py

math_substitutes = [
    ...
    ('\\le', '<='),
    ...
]

comment:3 Changed 8 years ago by aapitzsch

One solution might be to replace '\\left' and '\\right' by '' before replacing \\le.

comment:4 Changed 8 years ago by Travis Scrimshaw

I agree, along with perhaps:

  • '\\lvert' and '\\rvert' by '|',
  • '\\ast' by *,
  • '\\bigr', '\\bigl', etc. by ''.

comment:5 Changed 8 years ago by John Palmieri

Branch: u/jhpalmieri/typo_causes_latex_error_in_indexed_generators

comment:6 Changed 8 years ago by John Palmieri

Commit: 5792ca9ca3dbc21bd688e288a766c8563874febd

Here's an attempt at a fix, along the lines suggested.


New commits:

5792ca9#17745: remove \\left and \\right when running detex

comment:7 Changed 8 years ago by John Palmieri

Authors: John Palmieri
Status: newneeds_review

comment:8 Changed 8 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw

LGTM, but could you add one more test with a \\left, a \\le, and a \\leq to make sure all 3 get converted correctly? Thanks.

Last edited 8 years ago by Travis Scrimshaw (previous) (diff)

comment:9 Changed 8 years ago by git

Commit: 5792ca9ca3dbc21bd688e288a766c8563874febdef864a4e87afbf9776d811fc1333809194ee4b55

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

ef864a417745: one more doctest

comment:10 Changed 8 years ago by John Palmieri

Sure, here you go. Eventually, we might run into the same problem with \\to or \\ge; at some point we might want to change the whole system so it uses regular expressions instead of just a plain text replace.


New commits:

ef864a417745: one more doctest

comment:11 Changed 8 years ago by Travis Scrimshaw

Status: needs_reviewpositive_review

Thanks. Probably, but we'll cross that bridge if we come to it.

comment:12 Changed 8 years ago by John Palmieri

Status: positive_reviewneeds_work

I realized that this won't work: it will turn \\leftarrow and \\rightarrow into arrow. I think regular expressions are the way to go.

comment:13 Changed 8 years ago by Travis Scrimshaw

How about then just putting \\leftarrow and \\rightarrow before \left and \right (sending them to <-- and --> respectively?

Last edited 8 years ago by Travis Scrimshaw (previous) (diff)

comment:14 Changed 8 years ago by John Palmieri

Because I don't think we should actually be trying to run a serious LaTeX-to-text conversion here, and that's what this is in danger of becoming. What about \leftrightarrow, \leftleftarrows, etc.?

comment:15 Changed 8 years ago by Travis Scrimshaw

I'm not so familiar with using regular expressions, so if you can make it more elegant that way, then +1 from me.

comment:16 Changed 8 years ago by git

Commit: ef864a4e87afbf9776d811fc1333809194ee4b5518c6a2771996c623276e9cc362c9eecda69e4c4e

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

18c6a2717745: use regular expressions

comment:17 Changed 8 years ago by John Palmieri

Here's a regular expression version.

comment:18 Changed 8 years ago by John Palmieri

Status: needs_workneeds_review

comment:19 Changed 8 years ago by aapitzsch

The current implementation replaces LaTeX commands in the examples section, too. See for example

sage: from sage.structure.indexed_generators import IndexedGenerators
sage: IndexedGenerators?

That's bad. Users might complain, that they don't get the results show in the examples.

P.S. The ticket description should be updated.

comment:20 in reply to:  19 Changed 8 years ago by John Palmieri

Replying to aapitzsch:

The current implementation replaces LaTeX commands in the examples section, too.

I think that's complicated to fix. We would need to detect when we're in an EXAMPLES or TESTS block and ignore the output, but otherwise process the output. I guess we can do that, but it's a little annoying to do perfectly. This really should be done for each of detex, process_dollars, process_extlinks, process_mathtt. So maybe it should actually be done in format:

if not in examples block:
    detex(next line)
    process_dollars(next_lines)
    ...

So everything would need to be rewritten with this logic in mind. Alternatively, each of those functions will duplicate some code which keeps track of whether it's processing a line in an examples block.

I think that really, functions which have LaTeX output in their EXAMPLES (or elsewhere in their docstrings), and if that LaTeX should not be processed, then they should have their docstrings decorated with nodetex. That's why nodetex is available. Maybe we should add it to the IndexedGenerators docstring.

P.S. The ticket description should be updated.

Please go ahead. What did you have in mind?

comment:21 Changed 8 years ago by git

Commit: 18c6a2771996c623276e9cc362c9eecda69e4c4ec413a14653f72e20eb6553459d002f97f6280551

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

c413a1417745: add nodetex to IndexedGenerators docstring

comment:22 Changed 8 years ago by Karl-Dieter Crisman

Probably this will also fix the followup comment at the ask.sagemath question:

sage: timeit("a = 2nb=131nfactor(a^b-1)", number=25)
  25 loops, best of 3: ... per loop

comment:23 Changed 8 years ago by git

Commit: c413a14653f72e20eb6553459d002f97f62805510065db912af0bebb7f1149089e3a1b5b7159cd97

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

c67dd3dMerge branch 'develop' into t/17745/typo_causes_latex_error_in_indexed_generators
0065db917745: add more instances of nodetex

comment:24 Changed 8 years ago by Karl-Dieter Crisman

And even more examples from the same place:

Another such issues appear for dirac_delta, heaviside, unit_step. A \left gets replaced by <=ft

comment:25 Changed 8 years ago by Travis Scrimshaw

Status: needs_reviewpositive_review

I think this is a good step in the right direction; most importantly, it fixes the issue at hand. So I'm going to give this a positive review.

comment:26 Changed 8 years ago by Volker Braun

Branch: u/jhpalmieri/typo_causes_latex_error_in_indexed_generators0065db912af0bebb7f1149089e3a1b5b7159cd97
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.