Opened 3 years ago
Closed 3 years ago
#17745 closed defect (fixed)
typo causes latex error in indexed generators
Reported by:  kcrisman  Owned by:  

Priority:  minor  Milestone:  sage6.5 
Component:  documentation  Keywords:  
Cc:  tscrim, nthiery  Merged in:  
Authors:  John Palmieri  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  0065db9 (Commits)  Commit:  0065db912af0bebb7f1149089e3a1b5b7159cd97 
Dependencies:  Stopgaps: 
Description
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 3 years ago by
comment:2 Changed 3 years ago by
Problem seems to be in src/sage/misc/sagedoc.py
math_substitutes = [ ... ('\\le', '<='), ... ]
comment:3 Changed 3 years ago by
One solution might be to replace '\\left'
and '\\right'
by ''
before replacing \\le
.
comment:4 Changed 3 years ago by
I agree, along with perhaps:
'\\lvert'
and'\\rvert'
by''
,'\\ast'
by*
,'\\bigr'
,'\\bigl'
, etc. by''
.
comment:5 Changed 3 years ago by
 Branch set to u/jhpalmieri/typo_causes_latex_error_in_indexed_generators
comment:6 Changed 3 years ago by
 Commit set to 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 3 years ago by
 Status changed from new to needs_review
comment:8 Changed 3 years ago by
 Reviewers set to 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.
comment:9 Changed 3 years ago by
 Commit changed from 5792ca9ca3dbc21bd688e288a766c8563874febd to ef864a4e87afbf9776d811fc1333809194ee4b55
Branch pushed to git repo; I updated commit sha1. New commits:
ef864a4  17745: one more doctest

comment:10 Changed 3 years ago by
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:
ef864a4  17745: one more doctest

comment:11 Changed 3 years ago by
 Status changed from needs_review to positive_review
Thanks. Probably, but we'll cross that bridge if we come to it.
comment:12 Changed 3 years ago by
 Status changed from positive_review to needs_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 3 years ago by
How about then just putting \\leftarrow
and \\rightarrow
before \left
and \right
(sending them to <
and >
respectively?
comment:14 Changed 3 years ago by
Because I don't think we should actually be trying to run a serious LaTeXtotext conversion here, and that's what this is in danger of becoming. What about \leftrightarrow
, \leftleftarrows
, etc.?
comment:15 Changed 3 years ago by
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 3 years ago by
 Commit changed from ef864a4e87afbf9776d811fc1333809194ee4b55 to 18c6a2771996c623276e9cc362c9eecda69e4c4e
Branch pushed to git repo; I updated commit sha1. New commits:
18c6a27  17745: use regular expressions

comment:17 Changed 3 years ago by
Here's a regular expression version.
comment:18 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:19 followup: ↓ 20 Changed 3 years ago by
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 3 years ago by
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 3 years ago by
 Commit changed from 18c6a2771996c623276e9cc362c9eecda69e4c4e to c413a14653f72e20eb6553459d002f97f6280551
Branch pushed to git repo; I updated commit sha1. New commits:
c413a14  17745: add nodetex to IndexedGenerators docstring

comment:22 Changed 3 years ago by
Probably this will also fix the followup comment at the ask.sagemath question:
sage: timeit("a = 2nb=131nfactor(a^b1)", number=25) 25 loops, best of 3: ... per loop
comment:23 Changed 3 years ago by
 Commit changed from c413a14653f72e20eb6553459d002f97f6280551 to 0065db912af0bebb7f1149089e3a1b5b7159cd97
comment:24 Changed 3 years ago by
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 3 years ago by
 Status changed from needs_review to positive_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 3 years ago by
 Branch changed from u/jhpalmieri/typo_causes_latex_error_in_indexed_generators to 0065db912af0bebb7f1149089e3a1b5b7159cd97
 Resolution set to fixed
 Status changed from positive_review to closed
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?