Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slashes disappear in docstrings #10624

Closed
novoselt opened this issue Jan 13, 2011 · 11 comments
Closed

Slashes disappear in docstrings #10624

novoselt opened this issue Jan 13, 2011 · 11 comments

Comments

@novoselt
Copy link
Member

Consider functions

def g1():
    r"""
    S \ E
    S \\ E
    S \\\ E
    S \\\\ E
    S \\\\\ E
    S \\\\\\ E
    S \\\\\\\ E
    """
    pass

def g2():
    r"""
    S \ E
    S \\ E
    S \\\ E
    S \\\\ E
    S \\\\\ E
    S \\\\\\ E
    S \\\\\\\ E    
    ``x``
    """
    pass

i.e. they have the same docstrings but the second one has a code block (the same happens with math blocks).

When I type g1? in the notebook, I get pretty much the docstring as it is written, except for a couple of extra blank lines on top. When I type g2?, I get

S E S E S \ E S \ E S \E S \E S \\ E x

on a single line. I don't think that the treatment of slashes should depend on the presence of extra blocks in the docstring.

While removing slashes may be done for "deLaTeXifying" purposes, it is actually done (at least partially) before LaTeX processing. The docstring of

def g3():
    r"""
    .. MATH::
        
        a \\ b

    .. MATH::
        
        c \\\ d
    """
    pass

in the notebook shows a and b on the same line while c and d on different. For HTML documentation the first block works as it should - a and b are on different lines.

This problem came up on #10479 in the math block of NefPartition?.

Component: documentation

Keywords: notebook help docstring

Reviewer: John Palmieri

Issue created by migration from https://trac.sagemath.org/ticket/10624

@novoselt
Copy link
Member Author

comment:1

Still present in Sage-5.1.beta0...

@jhpalmieri
Copy link
Member

comment:2

In Sage 5.4.beta1, g3 doesn't look as described here: a and b are on different lines, and the same for c and d. I'm not sure how the triple back slashes are supposed to end up anyway.

For g1 vs. g2: in the case of g2, the docstring appears to be Sphinx markup (because of the backquotes — see the function is_sphinx_markup in devel/sagenb/sagenb/misc/sphinxify.py), so the function sphinxify is run on it, so the slashes get modified. I'm not sure what output you expect in this case, by the way; new lines in documentation should not be respected unless they are in a code block.

We could try to treat all docstrings as Sphinx markup instead. I can't think of another good global solution. For any particular docstring, you can force it to be processed by sphinxify by adding blah in the docstring somewhere. (For example, "the variable x should be ...")

@novoselt
Copy link
Member Author

comment:3

That's nice that there is an improvement: Sage 5.4.beta0 still shows a and b on the same line while c and d are shown on different as if there was a double slash between them.

For g1 and g2 I want the same formatting, so I think that all docstrings should be treated as if they were in Sphinx markup. And the major point is that slashes are treated in a wild unclear way - I understand that they are special characters and something happens to them, but the usual way is to replace double slashes with single ones and non-recursively, i.e. 4 slashes should become 2 and so on. Otherwise it is impossible to produce multiline math.

@novoselt
Copy link
Member Author

comment:4

I've just installed beta1: for me g3? still shows a and b on the same line and if I click to the left of the documentation, I get

    \[a \ b\]
    \[\begin{split}c \\ d\end{split}\]

so the double slash got replaced with a single one and the triple one with double one.

@novoselt
Copy link
Member Author

comment:5

OK, I think this is what has to be done.

  1. sagenb/sagenb/misc/support.py
   if is_sphinx_markup(s):
       try:
           return sphinxify(s)
       except:
           pass

There should be no if: try to sphinxify everything for uniform look. Since all functions are required to have examples now, this is happening anyway almost always.

  1. sagenb/sagenb/misc/sphinxiphy.py
   # This is needed for MathJax to work.
   docstring = docstring.replace('\\\\', '\\')

Get rid of this - it interferes with multiline math, there is no explanation why this is needed and it seems to work just fine without. I also suspect this MathJax here is an automatic replacement for jsMath, which may have had some issues.

  1. SAGE_ROOT/local/lib/python/site-packages/Sphinx-1.1.2-py2.7.egg/sphinx/ext/mathjax.py
   app.add_config_value('mathjax_inline', [r'\(', r'\)'], 'html')
   app.add_config_value('mathjax_display', [r'\[', r'\]'], 'html')

These are in the end and I think these parentheses have to go or the parsing should not refer to them at all. Otherwise math is wrapped twice: once in span/div with class="math" and inside them in parentheses, which I think should indicate math wrapping in "common text".

I am not sure where this change has to happen and whether it is a bug in Sphinx or Sage.

If someone puts these ideas to actual patches or explains how to do it, I will greatly appreciate it.

@jhpalmieri
Copy link
Member

comment:6

Re your third suggestion: I would be concerned that this would break regular documentation building. Have you tested it (say with sage --docbuild tutorial html)?

I'll try to look into this and related tickets (like #13455) soon. By the way, another possible change: in sage/sage/misc/latex_macros.py, there is the line

    defn = macro[start_defn+1: end_defn].replace('\\', '\\\\')

I wonder if the replace part should be deleted.

@jhpalmieri
Copy link
Member

comment:7

See sagemath/sagenb#97 for the Sagenb pull request. I think we need to keep the if is_sphinx_markup() block because the Sagenb project should function if Sphinx is not available. However, if it is, then that function should always return True, so that's how I've implemented it.

Instead of your item 3, I added a few replace('\\(', '') and similar to the code. These seem to clean things up for me, and they don't break docbuilding.

Should we close this ticket and move discussion to Sagenb at github?

@novoselt
Copy link
Member Author

comment:8

It seems silly to have a test which is always true, why can't we remove if line but keep the try block to deal with the absence of Sphinx?

Just deleting wrappers feels wrong as it is likely to become a bug once "the real bug" is fixed... But I have no better solution and documentation has to look good now.

Will try to figure out reviewing on github!

@novoselt novoselt removed this from the sage-5.4 milestone Sep 17, 2012
@jhpalmieri
Copy link
Member

comment:10

The latest version on github now just deletes the function is_sphinx_markup altogether.

@jdemeyer
Copy link

jdemeyer commented Oct 3, 2012

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:12

I don't think that our proposed changes to sagenb are a good idea anymore. Problems arise when looking at docstrings from other components of Sage; for example, if you do r.lm? in the notebook, before the patch here, newlines are respected, so the examples look good. But after the patch, the whole docstring is treated as ReST markup, and so the newlines are ignored and the examples look bad.

So we should keep working on this (at the sagenb github site, not here), but we need to have a better solution about when to use Sphinx and when not to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants