py3: fix src/sage/misc/nested_class.pyx

[chapoton]

traceback:

sage -t --long src/sage/misc/nested_class.pyx
**********************************************************************
File "src/sage/misc/nested_class.pyx", line 39, in sage.misc.nested_class
Failed example:
    A1.A2.A3
Expected:
    <class sage.misc.nested_class.A3 at ...>
Got:
    <class 'sage.misc.nested_class.A1.A2.A3'>
**********************************************************************
File "src/sage/misc/nested_class.pyx", line 42, in sage.misc.nested_class
Failed example:
    nested_pickle(A1)
Expected:
    <class sage.misc.nested_class.A1 at ...>
Got:
    <class 'sage.misc.nested_class.A1'>
**********************************************************************
File "src/sage/misc/nested_class.pyx", line 45, in sage.misc.nested_class
Failed example:
    A1.A2
Expected:
    <class sage.misc.nested_class.A1.A2 at ...>
Got:
    <class 'sage.misc.nested_class.A1.A2'>
**********************************************************************
File "src/sage/misc/nested_class.pyx", line 48, in sage.misc.nested_class
Failed example:
    A1.A2.A3
Expected:
    <class sage.misc.nested_class.A1.A2.A3 at ...>
Got:
    <class 'sage.misc.nested_class.A1.A2.A3'>
**********************************************************************
File "src/sage/misc/nested_class.pyx", line 131, in sage.misc.nested_class.modify_for_nested_pickle
Failed example:
    getattr(module, 'A.B', 'Not found')
Expected:
    <class '__main__.X.A.B'>
Got:
    <class '__main__.A.B'>
**********************************************************************
File "src/sage/misc/nested_class.pyx", line 133, in sage.misc.nested_class.modify_for_nested_pickle
Failed example:
    getattr(module, 'X.A.B', 'Not found')
Expected:
    <class '__main__.X.A.B'>
Got:
    <class '__main__.A.B'>
**********************************************************************
File "src/sage/misc/nested_class.pyx", line 219, in sage.misc.nested_class.nested_pickle
Failed example:
    getattr(module, 'A.B', 'Not found')
Expected:
    <class __main__.A.B at ...>
Got:
    <class '__main__.A.B'>
**********************************************************************
File "src/sage/misc/nested_class.pyx", line 259, in sage.misc.nested_class.NestedClassMetaclass
Failed example:
    A3.B.__name__
Expected:
    'A3.B'
Got:
    'B'
**********************************************************************
File "src/sage/misc/nested_class.pyx", line 261, in sage.misc.nested_class.NestedClassMetaclass
Failed example:
    getattr(sys.modules['__main__'], 'A3.B', 'Not found')
Expected:
    <class '__main__.A3.B'>
Got:
    'Not found'
**********************************************************************
4 items had failures:
   4 of  14 in sage.misc.nested_class
   2 of   6 in sage.misc.nested_class.NestedClassMetaclass
   2 of  23 in sage.misc.nested_class.modify_for_nested_pickle
   1 of   9 in sage.misc.nested_class.nested_pickle
    [68 tests, 9 failures, 1.77 s]

[nbruin]

To get the same repr for a class (without "at ...") in both Py2 and Py3, it may be as simple as defining the classes with class A(object):. It looks like the difference in printing is between old-style and new-style classes, and python3 has only new style classes. So, including the (object) brings the example in Py2 closer to what it does in Py3.

The problem with "name" might need more work.

[embray]

At some point I definitely fixed or changed something related to this but I'm wracking my brain to remember where.

[embray]

IIRC one of the main issues here is that I (?) fixed pickling of nested classes such that NestedClassMetaclass is all but unnecessary. But I might have dreamed it; it's all a bit foggy.

[embray]

Here it is--it turns out that the issue this is trying to work around just isn't relevant on Python 3, so it makes NestedClassMetaclass a no-op to be removed whenever we drop Python 2 support.

src/sage/misc/nested_class.pyx

@@ -36 +36 @@
 
     sage: A1.A2.A3.__name__
     'A3'
-    sage: A1.A2.A3
+    sage: A1.A2.A3  # py2
     <class sage.misc.nested_class.A3 at ...>
 
-    sage: nested_pickle(A1)
+    sage: nested_pickle(A1)  # py2
     <class sage.misc.nested_class.A1 at ...>
 
-    sage: A1.A2
+    sage: A1.A2  # py2
     <class sage.misc.nested_class.A1.A2 at ...>
+    sage: A1.A2  # py3 - note: here we have he desired name for free
+    <class 'sage.misc.nested_class.A1.A2'>
 
-    sage: A1.A2.A3
+    sage: A1.A2.A3  # py2
     <class sage.misc.nested_class.A1.A2.A3 at ...>
-    sage: A1.A2.A3.__name__
+    sage: A1.A2.A3  # py3
+    <class 'sage.misc.nested_class.A1.A2.A3'>
+    sage: A1.A2.A3.__name__  # py2
     'A1.A2.A3'
 
-    sage: sage.misc.nested_class.__dict__['A1.A2'] is A1.A2
+    sage: sage.misc.nested_class.__dict__['A1.A2'] is A1.A2  # py2
     True
-    sage: sage.misc.nested_class.__dict__['A1.A2.A3'] is A1.A2.A3
+    sage: sage.misc.nested_class.__dict__['A1.A2.A3'] is A1.A2.A3  # py2
     True
 
 All of this is not perfect. In the following scenario::
@@ -85 +89 @@
     giving them a mangled name and putting the mangled name in the
     module namespace.
 
+    On Python 3 this is a no-op and does not perform any mangling.
+
     INPUT:
 
     - ``cls`` - The class to modify.
@@ -111 +117 @@
         sage: getattr(module, 'A.B', 'Not found')
         'Not found'
         sage: modify_for_nested_pickle(A, 'A', sys.modules['__main__'])
-        sage: A.B.__name__
+        sage: A.B.__name__  # py2
         'A.B'
-        sage: getattr(module, 'A.B', 'Not found')
+        sage: getattr(module, 'A.B', 'Not found')  # py2
         <class '__main__.A.B'>
 
     Here we demonstrate the effect of the ``first_run`` argument::
 
         sage: modify_for_nested_pickle(A, 'X', sys.modules['__main__'])
-        sage: A.B.__name__ # nothing changed
+        sage: A.B.__name__ # py2: nothing changed
         'A.B'
         sage: modify_for_nested_pickle(A, 'X', sys.modules['__main__'], first_run=False)
-        sage: A.B.__name__
+        sage: A.B.__name__  # py2
         'X.A.B'
 
     Note that the class is now found in the module under both its old and
     its new name::
 
-        sage: getattr(module, 'A.B', 'Not found')
+        sage: getattr(module, 'A.B', 'Not found')  # py2
         <class '__main__.X.A.B'>
-        sage: getattr(module, 'X.A.B', 'Not found')
+        sage: getattr(module, 'X.A.B', 'Not found')  # py2
         <class '__main__.X.A.B'>
 
 
@@ -151 +157 @@
 
     Before :trac:`9107`, the name of ``A1.B1.C1`` would have been wrong::
 
-        sage: A1.B1.C1.__name__
+        sage: A1.B1.C1.__name__  # py2
         'A1.B1.C1'
-        sage: A1.B2.C2.__name__
+        sage: A1.B2.C2.__name__  # py2
         'A1.B2.C2'
         sage: A_module = sys.modules[A1.__module__]
-        sage: getattr(A_module, 'A1.B1.C1', 'Not found').__name__
+        sage: getattr(A_module, 'A1.B1.C1', 'Not found').__name__  # py2
         'A1.B1.C1'
-        sage: getattr(A_module, 'A1.B2.C2', 'Not found').__name__
+        sage: getattr(A_module, 'A1.B2.C2', 'Not found').__name__  # py2
         'A1.B2.C2'
 
     """
+    IF PY_MAJOR_VERSION >= 3:
+        return
+
     cdef str name, dotted_name
     cdef str mod_name = module.__name__
     cdef str cls_name = cls.__name__+'.'
@@ -214 +223 @@
     then the name of class ``"B"`` will be modified to ``"A.B"``, and the ``"A.B"``
     attribute of the module will be set to class ``"B"``::
 
-        sage: A.B.__name__
+        sage: A.B.__name__  # py2
         'A.B'
-        sage: getattr(module, 'A.B', 'Not found')
+        sage: getattr(module, 'A.B', 'Not found')  # py2
         <class __main__.A.B at ...>
 
     In Python 2.6, decorators work with classes; then ``@nested_pickle``
@@ -245 +254 @@
     r"""
     A metaclass for nested pickling.
 
+    On Python 3 this is just a direct wrapper around the base `type` and does
+    not do anything.
+
     Check that one can use a metaclass to ensure nested_pickle
     is called on any derived subclass::
 
@@ -256 +268 @@
         ....:     class B(object):
         ....:         pass
         ...
-        sage: A3.B.__name__
+        sage: A3.B.__name__  # py2
         'A3.B'
-        sage: getattr(sys.modules['__main__'], 'A3.B', 'Not found')
+        sage: getattr(sys.modules['__main__'], 'A3.B', 'Not found')  # py2
         <class '__main__.A3.B'>
     """
+
     def __init__(self, *args):
         r"""
         This invokes the nested_pickle on construction.
@@ -273 +286 @@
         ...
         sage: A.B
         <class '__main__.A.B'>
-        sage: getattr(sys.modules['__main__'], 'A.B', 'Not found')
+        sage: getattr(sys.modules['__main__'], 'A.B', 'Not found')  # py2
         <class '__main__.A.B'>
         """
         modify_for_nested_pickle(self, self.__name__, sys_modules[self.__module__])
@@ -306 +319 @@
                 sage: from sage.misc.nested_class import *
                 sage: loads(dumps(MainClass.NestedClass.NestedSubClass()))
                 <sage.misc.nested_class.MainClass.NestedClass.NestedSubClass object at 0x...>
-                sage: getattr(sage.misc.nested_class, 'MainClass.NestedClass.NestedSubClass')
+                sage: getattr(sage.misc.nested_class, 'MainClass.NestedClass.NestedSubClass')  # py2
                 <class 'sage.misc.nested_class.MainClass.NestedClass.NestedSubClass'>
-                sage: MainClass.NestedClass.NestedSubClass.__name__
+                sage: MainClass.NestedClass.NestedSubClass.__name__  # py2
                 'MainClass.NestedClass.NestedSubClass'
             """
             def dummy(self, x, *args, r=(1,2,3.4), **kwds):
@@ -321 +334 @@
                     sage: from sage.misc.nested_class import MainClass
                     sage: print(MainClass.NestedClass.NestedSubClass.dummy.__doc__)
                     NestedSubClass.dummy(self, x, *args, r=(1, 2, 3.4), **kwds)
-                    File: sage/misc/nested_class.pyx (starting at line 314)
+                    File: sage/misc/nested_class.pyx (starting at line 327)
                     <BLANKLINE>
                                     A dummy method to demonstrate the embedding of
                                     method signature for nested classes.

[embray]

And most of the tests are skipped on Python 3 as they are not relevant.

[chapoton]

branch please

[embray]

Here you go. With this, all the tests for src/sage/misc/nested_class.pyx pass on Python 3 (though many of them are simply skipped as inapplicable).

There might be some other minor failures related to this in miscellaneous modules, but I am not sure. Everything in python3-known-passing.txt still passes.

---

New commits:

​c876038

py3: Make NestedClassMetaclass do nothing on Python 3

[chapoton]

one failing doctest in src/sage/misc/sageinspect.py, see patchbot

[chapoton]

I fixed the doctest

---

New commits:

​a4e9eb3	Merge branch 'u/embray/python3/ticket-27692' in 8.8.b3
​a902cb2	fix doctest in sageinspect

[embray]

Right...there's always one of those...

[embray]

New commits:

​174e895

py3: Make NestedClassMetaclass do nothing on Python 3

[chapoton]

ok, great. Thx

[vbraun]

This ticket breaks docbuild on python3 (only); I don't know why but it does. Error is:

$ make doc-clean && make -j40 && make doc-html
[...]
[dochtml] Error building the documentation.
[dochtml] multiprocessing.pool.RemoteTraceback: 
[dochtml] """
[dochtml] Traceback (most recent call last):
[dochtml]   File "/var/lib/buildbot/slave/sage3_git/build/local/lib/python3.7/multiprocessing/pool.py", line 121, in worker
[dochtml]     result = (True, func(*args, **kwds))
[dochtml]   File "/var/lib/buildbot/slave/sage3_git/build/local/lib/python3.7/multiprocessing/pool.py", line 44, in mapstar
[dochtml]     return list(map(*args))
[dochtml]   File "/var/lib/buildbot/slave/sage3_git/build/local/lib/python3.7/site-packages/sage_setup/docbuild/__init__.py", line 79, in build_ref_doc
[dochtml]     getattr(ReferenceSubBuilder(doc, lang), format)(*args, **kwds)
[dochtml]   File "/var/lib/buildbot/slave/sage3_git/build/local/lib/python3.7/site-packages/sage_setup/docbuild/__init__.py", line 761, in _wrapper
[dochtml]     getattr(DocBuilder, build_type)(self, *args, **kwds)
[dochtml]   File "/var/lib/buildbot/slave/sage3_git/build/local/lib/python3.7/site-packages/sage_setup/docbuild/__init__.py", line 133, in f
[dochtml]     runsphinx()
[dochtml]   File "/var/lib/buildbot/slave/sage3_git/build/local/lib/python3.7/site-packages/sage_setup/docbuild/sphinxbuild.py", line 314, in runsphinx
[dochtml]     sys.stderr.raise_errors()
[dochtml]   File "/var/lib/buildbot/slave/sage3_git/build/local/lib/python3.7/site-packages/sage_setup/docbuild/sphinxbuild.py", line 249, in raise_errors
[dochtml]     raise OSError(self._error)
[dochtml] OSError: /var/lib/buildbot/slave/sage3_git/build/local/lib/python3.7/site-packages/sage/combinat/ncsf_qsym/qsym.py:docstring of sage.combinat.ncsf_qsym.qsym.QuasiSymmetricFunctions.Fundamental.Eulerian:55: WARNING: Duplicate explicit target name: "sw2010".

---

New commits:

​174e895

py3: Make NestedClassMetaclass do nothing on Python 3

[embray]

Not sure why I can't just progress straight to needs_work...

[embray]

I suppose it's possible there's some strange interaction between this, and how Sphinx treats nested classes (either in plain Sphinx, or something particular that Sage does with it...)

[jhpalmieri]

Or how Sphinx treats cached methods? A bandaid (untested) might be to move the citation out of qsym.py to the master bibliography file.

[jhpalmieri]

In fact that file has its own bibliography at the start, so this change might fix it:

src/sage/combinat/ncsf_qsym/qsym.py

@@ -62 +62 @@
    Quasisymmetric Functions Expand Positively into Young Quasisymmetric
    Schur Functions*. :arxiv:`1606.03519`
 
+.. [SW2010] John Shareshian and Michelle Wachs.
+   *Eulerian quasisymmetric functions*. (2010).
+   :arxiv:`0812.0764v2`
+
 AUTHOR:
 
 - Jason Bandlow
@@ -2348 +2352 @@
             - ``k`` -- (optional) if specified, determines the number of fixed
               points of the permutations which are being summed over
 
-            REFERENCES:
-
-            .. [SW2010] John Shareshian and Michelle Wachs.
-               *Eulerian quasisymmetric functions*. (2010).
-               :arxiv:`0812.0764v2`
-
             EXAMPLES::
 
                 sage: F = QuasiSymmetricFunctions(QQ).F()

Again, this is just a bandaid.

[fbissey]

Good job on Volker to find it out. I couldn't figure which ticket was causing that breakage.

[embray]

Hasn't there been some effort I've seen elsewhere to move bibliographies to module top-levels anyways (probably in part to prevent problems like this)?

[embray]

Or, I guess maybe I'm thinking of #27497, which is not exactly as I described.

[jhpalmieri]

The preferred thing is to move all references to the master bibliography file. combinat and graphs are the two directories remaining where this needs to be done.

[jhpalmieri]

I think my proposed change in comment:21 fixes the docbuilding problem. Should we incorporate it since it's the right thing to do to collect all of the references, or is it indicative of a further problem that needs investigating? (Do we care how Sphinx handles nested and/or cached methods, if that is indeed the problem?)

[embray]

Replying to jhpalmieri:

I think my proposed change in comment:21 fixes the docbuilding problem. Should we incorporate it since it's the right thing to do to collect all of the references, or is it indicative of a further problem that needs investigating? (Do we care how Sphinx handles nested and/or cached methods, if that is indeed the problem?)

On one hand, it might be indicative of a deeper problem; on the other hand I don't think it's something I want to place high priority on. I agree that this change fixes the problem for now. I just want to confirm that the compiled docs do still look as expected and then I'll update this branch and set it back to positive review.

[embray]

There does appear to be a legitimate issue here w.r.t. Sphinx.

In the class QuasiSymmetricFunctions the nested class Fundamental also has an alias of just F. That is, in the class body it has:

F = Fundamental

just as a shortcut. This does not affect the qualified name of the class, which is still correct:

sage: QuasiSymmetricFunctions.Fundamental.__qualname__
'QuasiSymmetricFunctions.Fundamental'

but for some reason Sphinx is outputting the docs for QuasiSymmetricFunctions with this nested class listed as just "F" instead of "Fundamental", whereas in the ​current docs it's listed correctly as "Fundamental".

[embray]

Back when I was last looking into this I was on the verge of tracking down the issue, but then everything went up in the air for a few weeks and now I'm not exactly sure where I was with this, though the issue did seem to be somewhere in one of Sage's Sphinx extensions.

[chapoton]

Is there any hope for progress here ? This is now the top-failing file, by number of failures.

[embray]

I think so. I (or someone) needs to work out exactly what's going on, IIRC in sage_autodoc, such that aliases for some nested classes are being preferred over the class itself.

I think it might just be some dict sorting issue, but it needs to actually explicitly detect cases where an object's attribute name is not the same as its __name__.

That is, when looping over the members of, for example, QuasiSymmetricFunctions, you'll find a Fundamental member and an F member which both point to the same object, the class named Fundamental. The docbuild is failing because this member is being listed as just "class F" and not "class Fundamental".

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​4214075

py3: Make NestedClassMetaclass do nothing on Python 3

[embray]

Rebased. Going to keep investigating the docbuild issue now.

[git]

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

​291d584

Move this reference to the module level.

[git]

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

​8337e67

Fix up sage_autodoc to work with normal Python 3 nested classes with qualnames,

[embray]

The above commit fixes the docbuild issue for me. The resulting docs now look as expected: QuasiSymmetricFunctions.Fundamental is documented as a nested class in QuasiSymmetricFunctions, while QuasiSymmetricFunctions.F is documented as an attribute, which is simply an alias for QuasiSymmetricFunctions.Fundamental.

Need to double-check that this still holds on Python 2 with this branch.

[embray]

I think this is good for Python 2 as well. Strangely, the .F alias does not get documented in the autodoc for the class on Python 2, and that appears to be the case in the current version of the docs as well. That difference is consistent before and after this change, whereas the Python 3 version seems better so I'm content to leave it at that.

[jhpalmieri]

This looks okay to me, but others should investigate, too.

[chapoton]

patchbot reports some pyflakes errors (easy to fix) and a docbuild failure:

+[dochtml] OSError: /home/sage-patchbot/sage/local/lib/python2.7/site-packages/sage/combinat/dyck_word.py:
docstring of sage.combinat.dyck_word.DyckWord_complete.pyramid_weight:23:
WARNING: Duplicate explicit target name: "ds1992".

[jhpalmieri]

After I fix the references in dyck_word.py, I get a ton of other "Duplicate citation" and "Duplicate explicit target names" from other files in sage/combinat. For example:

[combinat ] /Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage/combinat/fully_packed_loop.py:docstring of sage.combinat.fully_packed_loop.FullyPackedLoops.Element:376: WARNING: Duplicate explicit target name: "propp2001".
[combinat ] /Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage/combinat/fully_packed_loop.py:docstring of sage.combinat.fully_packed_loop.FullyPackedLoops.Element:381: WARNING: Duplicate explicit target name: "striker2015".
[combinat ] /Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage/combinat/fully_packed_loop.py:docstring of sage.combinat.fully_packed_loop.FullyPackedLoops.Element.gyration:9: WARNING: Duplicate explicit target name: "wieland00".
[combinat ] /Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage/combinat/fully_packed_loop.py:docstring of sage.combinat.fully_packed_loop.FullyPackedLoops.Element:376: WARNING: duplicate citation Propp2001, other instance in /Users/palmieri/Desktop/Sage_stuff/git/sage/src/doc/en/reference/combinat/sage/combinat/fully_packed_loop.rst
[combinat ] /Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage/combinat/fully_packed_loop.py:docstring of sage.combinat.fully_packed_loop.FullyPackedLoops.Element:381: WARNING: duplicate citation Striker2015, other instance in /Users/palmieri/Desktop/Sage_stuff/git/sage/src/doc/en/reference/combinat/sage/combinat/fully_packed_loop.rst
[combinat ] /Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage/combinat/fully_packed_loop.py:docstring of sage.combinat.fully_packed_loop.FullyPackedLoops.Element.gyration:9: WARNING: duplicate citation Wieland00, other instance in /Users/palmieri/Desktop/Sage_stuff/git/sage/src/doc/en/reference/combinat/sage/combinat/fully_packed_loop.rst
[combinat ] /Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage/combinat/interval_posets.py:docstring of sage.combinat.interval_posets.TamariIntervalPosets_all.Element.cubical_coordinates:14: WARNING: Duplicate explicit target name: "combe2019".
[combinat ] /Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/python2.7/site-packages/sage/combinat/interval_posets.py:docstring of sage.combinat.interval_posets.TamariIntervalPosets_all.Element.cubical_coordinates:14: WARNING: duplicate citation Combe2019, other instance in /Users/palmieri/Desktop/Sage_stuff/git/sage/src/doc/en/reference/combinat/sage/combinat/interval_posets.rst

[jhpalmieri]

This fixes the pyflakes errors:

src/sage_setup/docbuild/ext/sage_autodoc.py

@@ -30 +30 @@
 import inspect
 import re
 import sys
-import warnings
 
 from docutils.statemachine import ViewList
-from six import PY2, iteritems, itervalues, text_type, class_types, string_types
+from six import PY2, itervalues, text_type, class_types, string_types
 
 import sphinx
-from sphinx.errors import ExtensionError
 from sphinx.ext.autodoc.importer import mock, import_object, get_object_members
-from sphinx.ext.autodoc.inspector import format_annotation
 from sphinx.locale import _, __
 from sphinx.pycode import ModuleAnalyzer
-from sphinx.errors import ExtensionError, PycodeError
+from sphinx.errors import PycodeError
 from sphinx.util import logging
 from sphinx.util import rpartition, force_decode
 from sphinx.util.docstrings import prepare_docstring
-from sphinx.util.inspect import Signature, isdescriptor, safe_getmembers, \
+from sphinx.util.inspect import isdescriptor, safe_getmembers, \
     safe_getattr, object_description, is_builtin_class_method, \
     isenumattribute, isclassmethod, isstaticmethod, getdoc
-from sphinx.util.inspect import getargspec
 
 from sage.misc.sageinspect import (sage_getdoc_original,
                                    sage_getargspec, isclassinstance,

or at least, it fixes the ones that ought to be fixed.

[embray]

Replying to jhpalmieri:

After I fix the references in dyck_word.py, I get a ton of other "Duplicate citation" and "Duplicate explicit target names" from other files in sage/combinat. For example:

I didn't have this problem before, so is it possible it was recently introduced? Perhaps it's just another citation that needs to be moved to the module-level, or the master citations list?

[jhpalmieri]

It may only appear if you do make doc-clean. My guess is that whatever is forcing us to move the references in dyck_word.py to the module-level is also going to force us to move other references. The references in FullyPackedLoop are in a class with the decoration @add_metaclass(InheritComparisonClasscallMetaclass). The reference Combe2019 is in a method with @cached_method. Do we have to move all references in classes, methods, with these decorations to the module-level?

[jhpalmieri]

Another issue: in tableau_tuple.py we have

from sage.combinat.tableau import (Tableau, ...)

and later

class TableauTuple(CombinatorialElement):
    ...
    Element = Tableau

and now Sphinx thinks that all of the references in the Element class are duplicates of those from Tableau. This seems to be fixed by moving all of the references to the master bibliography file.

[jhpalmieri]

I'm getting some doctest failures with Python 3. Does anyone else see them?

----------------------------------------------------------------------
sage -t --long src/sage/categories/category.py  # 5 doctests failed
sage -t --long src/sage/categories/primer.py  # 1 doctest failed
sage -t --long src/sage/categories/category_with_axiom.py  # 5 doctests failed
sage -t --long src/sage/categories/homsets.py  # 1 doctest failed
sage -t --long src/sage/categories/polyhedra.py  # 1 doctest failed
----------------------------------------------------------------------

For example:

sage -t --long src/sage/categories/homsets.py
**********************************************************************
File "src/sage/categories/homsets.py", line 56, in sage.categories.homsets.HomsetsCategory.default_super_categories
Failed example:
    type(H)
Expected:
    <class 'sage.categories.additive_monoids.AdditiveMonoids.Homsets_with_category'>
Got:
    <class 'sage.categories.additive_monoids.Homsets_with_category'>

---

New commits:

​6ce1df6	trac 27692: pyflakes cleanup in sage_autodoc.py
​4475f30	trac 27692: move some references to master bibliography file

[chapoton]

I can confirm the same failure in categories/ with the present branch here.

[chapoton]

This has doctests failure with python3

[embray]

Moving tickets from the Sage 8.8 milestone that have been actively worked on in the last six months to the next release milestone (optimistically).

[chapoton]

Is there any hope for progress here ?

EDIT: the branch is now red

[chapoton]

I have made #28105 for just moving the references.

[chapoton]

Erik, John ? The branch is now red. Is there any hope to make at least a partial progress on this file ?

[jhpalmieri]

Replying to embray:

There does appear to be a legitimate issue here w.r.t. Sphinx.

In the class QuasiSymmetricFunctions the nested class Fundamental also has an alias of just F. That is, in the class body it has:

F = Fundamental

just as a shortcut. This does not affect the qualified name of the class, which is still correct:

sage: QuasiSymmetricFunctions.Fundamental.__qualname__
'QuasiSymmetricFunctions.Fundamental'

but for some reason Sphinx is outputting the docs for QuasiSymmetricFunctions with this nested class listed as just "F" instead of "Fundamental", whereas in the ​current docs it's listed correctly as "Fundamental".

I don't see this problem, at least with a Python 3 build. I see

F
    alias of QuasiSymmetricFunctions.Fundamental

class Fundamental(QSym)
    ...

Without this branch, the first two lines are missing, but everything else looks the same.

[jhpalmieri]

On the other hand, with this branch I see a new py3 doctest failure:

File "src/sage/misc/c3_controlled.pyx", line 449, in sage.misc.c3_controlled.CmpKey
Failed example:
    Sets().Infinite()._cmp_key
Expected:
    (4, ...)
Got:
    (0, 40)
**********************************************************************
1 item had failures:
   1 of  27 in sage.misc.c3_controlled.CmpKey

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​67f6132	py3: Make NestedClassMetaclass do nothing on Python 3
​5f63e7d	Fix up sage_autodoc to work with normal Python 3 nested classes with qualnames,
​e8a9b9f	trac 27692: pyflakes cleanup in sage_autodoc.py

[jhpalmieri]

Rebased to 8.9.beta2.

[jhpalmieri]

In my opinion, if the new doctest failure in c3_controlled.pyx can be fixed, then this is ready to go. I don't know what's causing it, though.

[chapoton]

This seems to be an issue with class name of Sets().Infinite()

in the flags tables, one sees

{'FacadeSets': 1,
 'FiniteSets': 2,
 'Sets.Infinite': 4,
 'EnumeratedSets': 8,

where the second line is the only line with a dot inside.

[jhpalmieri]

It may be more complicated than that. I made this change:

src/sage/misc/c3_controlled.pyx

@@ -375 +375 @@
 ##############################################################################
 
 cdef tuple atoms = ("FacadeSets",
-                    "FiniteSets", "Sets.Infinite", "EnumeratedSets", "SetsWithGrading",
+                    "FiniteSets", "InfiniteSets", "EnumeratedSets", "SetsWithGrading",
                     "Posets", "LatticePosets", "Crystals", "AdditiveMagmas",
                     "FiniteDimensionalModules", "GradedModules", "ModulesWithBasis",
                     "Magmas", "Semigroups", "Monoids", "PermutationGroups",

and then added a statement to print flags. I got this:

{'FacadeSets': 1, 'FiniteSets': 2, 'InfiniteSets': 4, 'EnumeratedSets': 8, 'SetsWithGrading': 16, 'Posets': 32, 'LatticePosets': 64, 'Crystals': 128, 'AdditiveMagmas': 256, 'FiniteDimensionalModules': 512, 'GradedModules': 1024, 'ModulesWithBasis': 2048, 'Magmas': 4096, 'Semigroups': 8192, 'Monoids': 16384, 'PermutationGroups': 32768, 'MagmasAndAdditiveMagmas': 65536, 'Rngs': 131072, 'Domains': 262144, 'HopfAlgebras': 524288}
sage: Sets().Finite()._cmp_key
(2, 55)
sage: Sets().Infinite()._cmp_key
(0, 40)
sage: Sets().Enumerated()._cmp_key
(8, 42)

So although InfiniteSets is now in flags with value of 4, the _cmp_key is still 0 for Sets().Infinite().

[jhpalmieri]

Without this branch:

sage: C = Sets().Infinite()
sage: C.__class__.__name__
'Sets.Infinite_with_category'

With this branch:

sage: C = Sets().Infinite()
sage: C.__class__.__name__
'Infinite_with_category'

Changing Sets.Infinite to just Infinite in atoms fixes the doctest, but it doesn't feel like the right thing to do.

[jhpalmieri]

This seems like a direct consequence of the change in nested_class.pyx. Do we do:

if PY2:
    ... define atoms using Sets.Infinite ...
else:
    ... define atoms using Infinite ...

Seems ugly.

[jhpalmieri]

For example:

src/sage/misc/c3_controlled.pyx

@@ -369 +369 @@
 from sage.misc.cachefunc import cached_function, cached_method
 from sage.misc.lazy_attribute import lazy_attribute
 from sage.structure.dynamic_class import dynamic_class
+from six import PY2
 
 ##############################################################################
 # Implementation of the total order between categories
 ##############################################################################
 
+if PY2:
+    infinitesets = "Sets.Infinite"
+else:
+    infinitesets = "Infinite"
+
 cdef tuple atoms = ("FacadeSets",
-                    "FiniteSets", "Sets.Infinite", "EnumeratedSets", "SetsWithGrading",
+                    "FiniteSets", infinitesets, "EnumeratedSets", "SetsWithGrading",
                     "Posets", "LatticePosets", "Crystals", "AdditiveMagmas",
                     "FiniteDimensionalModules", "GradedModules", "ModulesWithBasis",
                     "Magmas", "Semigroups", "Monoids", "PermutationGroups",

I don't like it, but it works.

[klee]

New commits:

​4214075	py3: Make NestedClassMetaclass do nothing on Python 3
​291d584	Move this reference to the module level.
​8337e67	Fix up sage_autodoc to work with normal Python 3 nested classes with qualnames,
​6ce1df6	trac 27692: pyflakes cleanup in sage_autodoc.py
​4475f30	trac 27692: move some references to master bibliography file
​78ab9c5	Merge branch 'develop'
​fb6d5fa	Keep NestedClassMetaClass functional even with Python 3

[klee]

In the commit fb6d5fa, I followed different approach to solve the issue. I keep NestedClassMetaclass as it was even with python3, but fixes doctests appropriately for both python2 and python3. See the note in the module docstring.

I think this approach is simple and the right one at this stage.

To a reviewer: I am not completely sure about what I wrote in the note. Please correct the note.

[jhpalmieri]

Tests pass for me. The approach seems okay to me, but I am not an expert on this aspect of the code. What do others think? @embray?

[klee]

I add that Erik's approach should be considered when we drop the support on python2 and remove the nested class module altogether.

[jhpalmieri]

It would be nice to improve the documentation. This is not a new addition, but what do "save_global" and "load_global" refer to? I guess they are methods in Python's pickle module, but they are undocumented, which makes them less than ideal as reference points. (Fixing this could certainly wait until another ticket.)

[jhpalmieri]

The only issue here is the note added by klee. Is it correct? If so, I think this is ready to go.

[git]

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

​04645f1

Correct the note

[klee]

Replying to jhpalmieri:

It would be nice to improve the documentation. This is not a new addition, but what do "save_global" and "load_global" refer to? I guess they are methods in Python's pickle module, but they are undocumented, which makes them less than ideal as reference points. (Fixing this could certainly wait until another ticket.)

They are C functions in cPickle module (actually cPickle.c), and not visible to a user.

[klee]

In the last commit, I corrected the note, after investigating cPickle of python2 and the pickle module of python3. So I am now sure that what it says is correct.

I think this ticket is now good to go.

[jhpalmieri]

Replying to klee:

Replying to jhpalmieri:

It would be nice to improve the documentation. This is not a new addition, but what do "save_global" and "load_global" refer to? I guess they are methods in Python's pickle module, but they are undocumented, which makes them less than ideal as reference points. (Fixing this could certainly wait until another ticket.)

They are C functions in cPickle module (actually cPickle.c), and not visible to a user.

Exactly. Not only not visible to a user, but as I said, not documented anywhere, so having them as part of our documentation is not helpful.

[git]

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

​040265b

Remove mentioning undocumented functions in cPickle

[klee]

Replying to jhpalmieri:

Replying to klee:

Replying to jhpalmieri:

It would be nice to improve the documentation. This is not a new addition, but what do "save_global" and "load_global" refer to? I guess they are methods in Python's pickle module, but they are undocumented, which makes them less than ideal as reference points. (Fixing this could certainly wait until another ticket.)

They are C functions in cPickle module (actually cPickle.c), and not visible to a user.

Exactly. Not only not visible to a user, but as I said, not documented anywhere, so having them as part of our documentation is not helpful.

Ok. Then let's remove them here.

[chapoton]

I think that it is more than time to move on here.

Given that John and Kwankyu said "good to go", I am setting to positive

[vbraun]

Now it fails on py2 with:

[dochtml] OSError: /var/lib/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/combinat/tableau.py:docstring of sage.combinat.tableau.Tableaux.Element.promotion:40: WARNING: Duplicate explicit target name: "hai1992".

[git]

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

​a1e3a4c	Merge branch 'develop' into t/27692/public/27692
​86df126	trac 27692: fix one citation

[jhpalmieri]

This should fix the problem.

[vbraun]

Now the pdf docs are broken

[docpdf] Underfull \hbox (badness 10000) in paragraph at lines 4766--4767
[docpdf] \T1/ptm/m/n/10 Bases: [][]\T1/pcr/m/n/10 sage.categories.category_with_axiom.
[docpdf] 
[docpdf] ! LaTeX Error: Too deeply nested.
[docpdf] 
[docpdf] See the LaTeX manual or LaTeX Companion for explanation.
[docpdf] Type  H <return>  for immediate help.
[docpdf]  ...                                              
[docpdf]                                                   
[docpdf] l.4779 \begin{itemize}
[docpdf]                       
[docpdf] ? 
[docpdf] ! Emergency stop.
[docpdf]  ...                                              

[git]

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

​13eb46f	Merge branch 'develop'
​fb30abb	Merge branch 'public/27692'
​fefe75e	Fix class documenter in sage_autodoc
​2f3fc35	Recover some original docstrings

[klee]

Seems to have fixed pdf docs.

[git]

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

​d8cc214

Make aliases documented

[klee]

The last commit makes aliases of nested classes documented.

It looks all good to me. But the nested class stuff of Sage is very subtle, and these changes might have unexpected effects at unexpected places in the documentation.

Please check the generated docs carefully, before we move on.

[fbissey]

Needs rebasing on top of 8.9.beta4.

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​562ff1e

Fix nested_class for python 3

[jhpalmieri]

With the new version, attributes appear in the reference manual. This is a change, but it is fine with me.

[jhpalmieri]

That's the only real difference, as far as I can tell. There may be some small differences in the indices (files like ​http://doc.sagemath.org/html/en/reference/genindex-A.html), as well, but very minor ones. I am happy with this.

(Edit: I built docs with both Python 2 and Python 3, with and without this branch, and ran diff -r ... on the resulting doc directories, so I could see all of the differences. I only checked the html and latex output.)

[klee]

I am also happy with more documentation. Just for the record, more members of a class, like attributes and aliases, now appear in the documentation because of a change in skip_member function defined in sage/docs/conf.py, which used to skip them for documentation:

     objname = getattr(obj, "__name__", None)
     if objname is not None:
-        if objname.find('.') != -1 and objname.split('.')[-1] != name:
-            return True
+        # check if name was inserted to the module by NestedClassMetaclass
+        if name.find('.') != -1 and objname.find('.') != -1:
+            if objname.split('.')[-1] == name.split('.')[-1]:
+                return True
 
     if name.find("userchild_download_worksheets.zip") != -1:
         return True