py3: HTML documentation for GlobalOptions does not show up correctly

[mwageringel]

With Python 3, the HTML documentation for GlobalOptions does not show the docstring, but something else. For example in the case of QQbar.options, this is what gets displayed:

options = Current options for AlgebraicField - display_format: decimal

This is what is called "String form" in the introspection QQbar.options?.

With Python 2, the HTML documentation shows the "Docstring" as desired:

options(*get_value, **set_value)
OPTIONS:

* "display_format" -- (default: "decimal")

  * "decimal" -- Always display a decimal approximation

  * "radical" -- Display using radicals (if possible)

See "GlobalOptions" for more features of these options.

This is not unique to QQbar, but can also be seen with Tableaux.options, for instance.

CC: @kwankyu

Component: python3

Author: Kwankyu Lee

Branch/Commit: 16b5e11

Reviewer: Markus Wageringel

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

[jhpalmieri]

comment:1

Can you provide more details? From the Sage command line or from the Jupyter notebook, if I type QQbar.options?, I see the same thing in Python 2 or Python 3.

[mwageringel]

comment:2

It is displayed correctly on the command line. The problem is with the HTML files that get generated as part of the documentation. Currently, the problem can be seen here online: Python 2/Python 3.

[egourgoulhon]

comment:3

I confirm the bug in Sage 9.0.beta4 (Python 3 version). It holds for Manifold.options as well.

[mwageringel]

comment:4

The following (non-sensical) change in sage_autodoc.AttributeDocumenter.can_document_member appears to fix this problem.

--- a/src/sage_setup/docbuild/ext/sage_autodoc.py
+++ b/src/sage_setup/docbuild/ext/sage_autodoc.py
@@ -1477,17 +1477,17 @@ class AttributeDocumenter(DocstringStripSignatureMixin, ClassLevelDocumenter):

         isattribute = isdatadesc or (not isinstance(parent, ModuleDocumenter) and isattr)

         # Trac #26522: This condition is here just to pass objects of classes
         # that inherit ClasscallMetaclass as attributes rather than method
         # descriptors.
         isattribute = isattribute or isinstance(type(member), ClasscallMetaclass)

-        if PY2:
+        if PY2 or True:
             return isattribute

         # That last condition addresses an obscure case of C-defined
         # methods using a deprecated type in Python 3, that is not otherwise
         # exported anywhere by Python
         return isattribute or (not isinstance(parent, ModuleDocumenter) and
                               not inspect.isroutine(member) and
                               not isinstance(member, type))

The problem is that sage_autodoc determines that QQbar.options should be documented as an attribute rather than a function, in Python 3, because the expression in the last return statement is true.

Note that class options(GlobalOptions) does not create a subclass, but an instance of GlobalOptions by some magic, so it acually is an attribute, which nevertheless should not be documented as such.

Also note that sage_autodoc is a modified copy of Sphinx' autodoc. The problematic expression has not been added by Sage, but is part of the upstream version. The PY2-check was added in #26949 when sage_autodoc was updated to be more in line with the upstream version (which is Python-3-only I assume).

Any ideas how to solve this? It would be good if we could avoid adding a special case for GlobalOptions and also avoid deviating from upstream too much.

[jhpalmieri]

comment:5

@kwankyu: any suggestions?

[kwankyu]

Author: Kwankyu Lee

[kwankyu]

Branch: u/klee/28698

[sagetrac-git]

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

16b5e11

Fix sage_auto_doc for GlobalOptions to appear correctly.

[sagetrac-git]

Commit: 16b5e11

[kwankyu]

comment:8

Replying to @mwageringel:

Any ideas how to solve this? It would be good if we could avoid adding a special case for GlobalOptions and also avoid deviating from upstream too much.

The problem is that GlobalOptions attribute get documented by AttributeDocumenter unlike by FunctionDocumenter in python 2. This is because of the extra check for "obscure case" in python 3.

I don't clearly understand what the "obscure case" is. Anyway, the extra check is apparently doing more harm than good to sage documentation. In the patch, I turned off that check, as you originally suggested. I expect the generated documentation is more close to the version in python 2.

We need to examine the generated documentation to see if there is any regression from the version in python 2.

[mwageringel]

comment:10

I ran a diff on the generated html files before and after the patch using Python 3. The change mainly affects all the options classes that are now documented as function rather than attribute.

Besides options, there are two or three cases where the documentation also switched from attribute to function, for example

FiniteStateMachine.default_format_letter

which is just an alias to an imported function

    default_format_letter = latex

so default_format_letter now shows the documentation from latex, which seems correct.

However, this patch also affects a number of attributes that completely disappear from the documentation after the patch, for example:

AbstractGrowthGroupFunctor.rank,
GenericGrowthGroup.CartesianProduct,
GenericSymbolicSubringFunctor.coercion_reversed,
CartesianProductFunctor.symbol,
A000027.link,
CNFEncoder.permutations

Are these supposed to appear in the documentation or not? Most of them do not have documentation strings attached to them, but some do (possibly unintentionally), for example:

FSMState.initial_probability

[kwankyu]

comment:11

Replying to @mwageringel:

I ran a diff on the generated html files before and after the patch using Python 3.

Thanks.

The change mainly affects all the options classes that are now documented as function rather than attribute.

Good.

Besides options, there are two or three cases where the documentation also switched from attribute to function, for example

FiniteStateMachine.default_format_letter

which is just an alias to an imported function

    default_format_letter = latex

so default_format_letter now shows the documentation from latex, which seems correct.

Seems ok.

However, this patch also affects a number of attributes that completely disappear from the documentation after the patch, for example:

AbstractGrowthGroupFunctor.rank,
GenericGrowthGroup.CartesianProduct,
GenericSymbolicSubringFunctor.coercion_reversed,
CartesianProductFunctor.symbol,
A000027.link,
CNFEncoder.permutations

Are these supposed to appear in the documentation or not?

They did not appear in python 2 documentation, but started to appear in python 3 documentation by #26949, perhaps because of the extra check in AttributeDocumenter. So I think they should not appear in the documentation. I regard this as an improvement.

Most of them do not have documentation strings attached to them, but some do (possibly unintentionally), for example:

FSMState.initial_probability

This example is interesting.

The attribute initial_probability did have docstring, but as of sage 9.0.beta4, the docstring is removed, or rather moved into the class (FSMState) docstring. This is why initial_probability (and is_initial) disappeared from the python 3 documentation.

Then why the docstring was moved to class docstring? Perhaps because the attribute docstrings do not get doctested. I think this is a defect of sage doctesting -- the original docstring was nice!

To summarize, I see no regression from your report.

[mwageringel]

Reviewer: Markus Wageringel

[mwageringel]

comment:13

Ok, thank you for the explanations. Considering that the documentation in Python 3 now seems to agree with the usual Python 2 documentation, this is good to go then.

[vbraun]

Changed branch from u/klee/28698 to 16b5e11