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

Python 3 vs. Sphinx, part 2: classes inheriting from ClasscallMetaclass #26522

Closed
jhpalmieri opened this issue Oct 21, 2018 · 27 comments
Closed

Comments

@jhpalmieri
Copy link
Member

(split off of #26449)

With a Python 3 build of Sage, Sphinx omits large chunks of the autodoc documentation. It seems to be related to classes which inherit from ClasscallMetaclass and which also have class-level docstrings. For example, if I have this:

class Test1(ClasscallMetaclass):
    def A(self):
        """
        documentation for A
        """
        pass

class Test2(ClasscallMetaclass):
    """
    class-level docstring
    """
    def B(self):
        """
        documentation for B
        """
        pass

then Sphinx will produce output looking like this:

class sage.algebras.commutative_dga.Test1
    Bases: sage.misc.classcall_metaclass.ClasscallMetaclass

    A()
        documentation for A

sage.algebras.commutative_dga.Test2

    class-level docstring

Test1 is properly documented, but Test2 is missing documentation for the methods; furthermore, it doesn't say "class" at the beginning and does not list the base class.

Classes which inherit from other classes, say WithEqualitybyId, work just fine.

CC: @fchapoton @jdemeyer @embray

Component: python3

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

@jhpalmieri jhpalmieri added this to the sage-8.5 milestone Oct 21, 2018
@jhpalmieri

This comment has been minimized.

@jhpalmieri jhpalmieri changed the title Python 3 vs. Sphinx, part 2 Python 3 vs. Sphinx, part 2: classes inheriting from UniqueRepresentation Oct 21, 2018
@jhpalmieri jhpalmieri changed the title Python 3 vs. Sphinx, part 2: classes inheriting from UniqueRepresentation Python 3 vs. Sphinx, part 2: classes inheriting from CachedRepresentation Oct 21, 2018
@jhpalmieri
Copy link
Member Author

comment:3

This is important to fix; otherwise, the Python 3 build of the reference manual will be very incomplete, missing methods from every class that inherits from CachedRepresentation. I also have no idea how to fix it.

@jhpalmieri
Copy link
Member Author

comment:5

This change doesn't help, but is there any reason not to do this:

diff --git a/src/sage/structure/unique_representation.py b/src/sage/structure/unique_representation.py
index 4fc44c8b86..e08393954b 100644
--- a/src/sage/structure/unique_representation.py
+++ b/src/sage/structure/unique_representation.py
@@ -561,13 +561,13 @@ accordingly, for example by inheriting from
 #******************************************************************************
 from __future__ import print_function
 
-from sage.misc import six
+from six import with_metaclass
 from sage.misc.cachefunc import weak_cached_function
 from sage.misc.classcall_metaclass import ClasscallMetaclass, typecall
 from sage.misc.fast_methods import WithEqualityById
 
 
-class CachedRepresentation(six.with_metaclass(ClasscallMetaclass)):
+class CachedRepresentation(with_metaclass(ClasscallMetaclass)):
     """
     Classes derived from CachedRepresentation inherit a weak cache for their
     instances.

@jhpalmieri

This comment has been minimized.

@jhpalmieri
Copy link
Member Author

comment:6

A little more experimentation has led me to change the ticket description. I think the problem is with ClasscallMetaclass, not CachedRepresentation. Furthermore, it only manifests itself when there is a class-level docstring.

@jhpalmieri jhpalmieri changed the title Python 3 vs. Sphinx, part 2: classes inheriting from CachedRepresentation Python 3 vs. Sphinx, part 2: classes inheriting from ClasscallMetaclass Nov 6, 2018
@jhpalmieri
Copy link
Member Author

comment:7

I should note that if I add methods to ClasscallMetaclass in sage.misc.classcall_metaclass (in particular, methods which do not start with an underscore), they are not included in the documentation, and the built documentation for ClasscallMetaclass does not list its base.

@dimpase
Copy link
Member

dimpase commented Dec 3, 2018

comment:8

This looks like very generic code which is not really even dependent upon Sage, so one can create a separate sphinx testcase for it, right?

@jhpalmieri
Copy link
Member Author

comment:9

If someone could figure out what's going wrong, that would be preferable to making a special case of it. Is it because ClasscallMetaclass is defined in a cython file? It must be more than that, so what makes things go wrong with this particular class?

@strogdon
Copy link

strogdon commented Dec 5, 2018

comment:10

On my sage-on-gentoo install I'm able to complete building the docs with python3 if I disable catching the OSError that normally terminates the build:

diff --git a/src/sage_setup/docbuild/sphinxbuild.py b/src/sage_setup/docbuild/sphinxbuild.py
index 8f7034fcb8..6cfdb10080 100644
--- a/src/sage_setup/docbuild/sphinxbuild.py
+++ b/src/sage_setup/docbuild/sphinxbuild.py
@@ -250,8 +250,8 @@ class SageSphinxLogger(object):
             OSError: This is a SEVERE error
 
         """
-        if self._error is not None:
-            raise OSError(self._error)
+#        if self._error is not None:
+#            raise OSError(self._error)
 
     _line_buffer = ''

Perhaps someone can try this on vanilla to see if it works with python3. If it works then perhaps we can determine which pieces are missing.

@strogdon
Copy link

strogdon commented Dec 5, 2018

comment:11

This is probably what @jhpalmieri was indicating. Under Special Methods for Classes

Correct doc

class sage.misc.classcall_metaclass.ClasscallMetaclass
Bases: sage.misc.nested_class.NestedClassMetaclass

A metaclass providing support for special methods for classes.

Incorrect with python3

 sage.misc.classcall_metaclass.ClasscallMetaclass

    A metaclass providing support for special methods for classes.

And perhaps other places.

@jhpalmieri
Copy link
Member Author

comment:12

For another illustration, navigate in the reference manual by choosing Categories and then Sets. The documentation for the class Sets in the Python 2 build includes lots of methods (for example
an_element), but the documentation for the Python 3 build only has the top-level docstring.

This is what actually causes the documentation to fail to build with Python 3: a cross-reference to "facade-sets" points to what becomes omitted in the Python 3 build of the documentation.

@strogdon
Copy link

strogdon commented Dec 7, 2018

comment:13

Also, python3 appears to not be picking up on external links. From the html code near the top of
Sets

python2:

<dd><p>Bases: <a class="reference external" href="https://docs.python.org/library/exceptions.html#exceptions.ValueError" title="(in Python v2.7)"><code class="xref py py-class docutils literal notranslate"><span class="pre">exceptions.ValueError</span></code></a></p>

python3:

<dd><p>Bases: <code class="xref py py-class docutils literal notranslate"><span class="pre">ValueError</span></code></p>

It's not clear to me where these external links originate.

@jhpalmieri
Copy link
Member Author

comment:14

I wonder if those links come from src/doc/common/python.inv. That file probably needs to be updated for Python 3. Maybe we need two different versions.

@jhpalmieri
Copy link
Member Author

comment:15

Confirmed: if I download python.inv using the script update-python-inv.sh (modified to make sure it gets the Python 3.6.6 version), then the link reappears.

@jhpalmieri
Copy link
Member Author

comment:16

See #26859 for the python.inv issue: I propose including files python2.inv and python3.inv.

@embray
Copy link
Contributor

embray commented Dec 10, 2018

comment:17

Replying to @jhpalmieri:

This change doesn't help, but is there any reason not to do this:

There is a bug in some versions of six that with_metaclass doesn't work properly with some classes (I think nested classes in particular). I think the version of six in sage-the-distribution has this fixed.

@jhpalmieri
Copy link
Member Author

comment:18

Replying to @embray:

Replying to @jhpalmieri:

This change doesn't help, but is there any reason not to do this:

There is a bug in some versions of six that with_metaclass doesn't work properly with some classes (I think nested classes in particular). I think the version of six in sage-the-distribution has this fixed.

In sage.misc.six, there is no with_metaclass. Maybe there used to be. It gets imported via from six import * at the top of sage.misc.six.

@strogdon
Copy link

comment:19

I'm wondering if there is a problem with the inventory builder. Here, for example, with python3 most everything is missing under 'Finite Coxeter Groups' including the .png on that page. The .png should be generated under local/share/doc/sage/inventory/en/reference/categories/sage/categories/. At least with my sage-on-gentoo build the .png file is not generated.

@jhpalmieri
Copy link
Member Author

comment:20

Note that FiniteCoxeterGroups inherits from CategoryWithAxiom which inherits from Category which inherits from UniqueRepresentation which inherits from ClasscallMetaclass. Maybe you're right: maybe the problem is with the inventory builder, or at least maybe that's where it first manifests itself.

@strogdon
Copy link

comment:21

Another data point. With Python3 numerous entries in inventory objects.inv files are assigned different sphinx directives from those assigned with Python2. For example, in the objects.inv file under local/share/doc/sage/inventory/en/reference/misc the entry

sage.misc.classcall_metaclass.ClasscallMetaclass sage/misc/classcall_metaclass.html#sage.misc.classcall_metaclass.ClasscallMetaclass

is assigned the py:class directive when using Python2 but the py:attribute directive when using Python3. I assume this will affect how things appear in html docs.

python -m sphinx.ext.intersphinx objects.inv | less

to view the file contents.

@jhpalmieri
Copy link
Member Author

comment:22

That's interesting. I don't understand anything about how Sphinx generates the inventory files. I've tried poking around, but I'm not getting anywhere.

@kwankyu
Copy link
Collaborator

kwankyu commented Dec 24, 2018

comment:23

Please check out #26949

@strogdon
Copy link

comment:24

Replying to @kwankyu:

Please check out #26949

@jhpalmieri:

I'll be interested in your comments. It probably would have been better to have the commits in this location but so be it. The html-docs do build for me but I haven't been able to view them. I do notice some DeprecationWarnings that I don't believe were present with python 2, things like

[repl     ] <unknown>:181: DeprecationWarning: invalid escape sequence \)
[repl     ] <unknown>:365: DeprecationWarning: invalid escape sequence \w
[matrices ] <unknown>:6657: DeprecationWarning: invalid escape sequence \p
[number_fi] <unknown>:6657: DeprecationWarning: invalid escape sequence \p
[rings_num] <unknown>:6657: DeprecationWarning: invalid escape sequence \p

@strogdon
Copy link

comment:25

There are subtle differences under Sets. We need a central place to discuss this.

@jhpalmieri
Copy link
Member Author

comment:26

Those invalid escape sequences crop up in the Python 3 docbuilding process. Other tickets have removed many of them.

@embray embray removed this from the sage-8.5 milestone Jan 10, 2019
@embray embray added the pending label Jan 10, 2019
@kwankyu
Copy link
Collaborator

kwankyu commented Jan 28, 2019

comment:28

Now fixed by #26949.

@embray
Copy link
Contributor

embray commented Jan 28, 2019

comment:29

Cool, thank you.

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

5 participants