Closed fchapoton closed 5 years ago
nbruin
commented
5 years ago 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
commented
5 years ago At some point I definitely fixed or changed something related to this but I'm wracking my brain to remember where.
embray
commented
5 years ago 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
commented
5 years ago 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.
diff --git a/src/sage/misc/nested_class.pyx b/src/sage/misc/nested_class.pyx
index 3cd3dfc..cf9c352 100644
--- a/src/sage/misc/nested_class.pyx
+++ b/src/sage/misc/nested_class.pyx
@@ -36,23 +36,27 @@ EXAMPLES::
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,6 +89,8 @@ cpdef modify_for_nested_pickle(cls, str name_prefix, module, first_run=True):
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,26 +117,26 @@ cpdef modify_for_nested_pickle(cls, str name_prefix, module, first_run=True):
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,17 +157,20 @@ cpdef modify_for_nested_pickle(cls, str name_prefix, module, first_run=True):
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,9 +223,9 @@ def nested_pickle(cls):
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,6 +254,9 @@ cdef class NestedClassMetaclass(type):
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,11 +268,12 @@ cdef class NestedClassMetaclass(type):
....: 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,7 +286,7 @@ cdef class NestedClassMetaclass(type):
...
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,9 +319,9 @@ class MainClass(object, metaclass=NestedClassMetaclass):
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,7 +334,7 @@ class MainClass(object, metaclass=NestedClassMetaclass):
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.And most of the tests are skipped on Python 3 as they are not relevant.
fchapoton
commented
5 years ago branch please
embray
commented
5 years ago 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 |
embray
commented
5 years ago Branch: u/embray/python3/ticket-27692
embray
commented
5 years ago Author: Erik Bray
fchapoton
commented
5 years ago one failing doctest in src/sage/misc/sageinspect.py, see patchbot
fchapoton
commented
5 years ago Changed branch from u/embray/python3/ticket-27692 to public/python3/ticket-27692
fchapoton
commented
5 years ago embray
commented
5 years ago Right...there's always one of those...
embray
commented
5 years ago New commits:
174e895 | py3: Make NestedClassMetaclass do nothing on Python 3 |
embray
commented
5 years ago Changed branch from public/python3/ticket-27692 to u/embray/python3/ticket-27692
fchapoton
commented
5 years ago ok, great. Thx
fchapoton
commented
5 years ago Reviewer: Frédéric Chapoton
vbraun
commented
5 years ago Changed branch from u/embray/python3/ticket-27692 to 174e895
vbraun
commented
5 years ago Changed branch from 174e895 to u/embray/python3/ticket-27692
vbraun
commented
5 years ago 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
commented
5 years ago Not sure why I can't just progress straight to needs_work...
embray
commented
5 years ago 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
commented
5 years ago 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
commented
5 years ago In fact that file has its own bibliography at the start, so this change might fix it:
diff --git a/src/sage/combinat/ncsf_qsym/qsym.py b/src/sage/combinat/ncsf_qsym/qsym.py
index ae7d830ed5..61004f24cd 100644
--- a/src/sage/combinat/ncsf_qsym/qsym.py
+++ b/src/sage/combinat/ncsf_qsym/qsym.py
@@ -62,6 +62,10 @@ REFERENCES:
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,12 +2352,6 @@ class QuasiSymmetricFunctions(UniqueRepresentation, Parent):
- ``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.
kiwifb
commented
5 years ago Good job on Volker to find it out. I couldn't figure which ticket was causing that breakage.
embray
commented
5 years ago 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
commented
5 years ago Or, I guess maybe I'm thinking of #27497, which is not exactly as I described.
jhpalmieri
commented
5 years ago 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
commented
5 years ago 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
commented
5 years ago 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
commented
5 years ago 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 = Fundamentaljust 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
commented
5 years ago 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.
fchapoton
commented
5 years ago Is there any hope for progress here ? This is now the top-failing file, by number of failures.
embray
commented
5 years ago 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".
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313
commented
5 years ago 7ed8c4ca-6d56-4ae9-953a-41e42b4ed313
commented
5 years ago 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
commented
5 years ago Rebased. Going to keep investigating the docbuild issue now.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313
commented
5 years ago 7ed8c4ca-6d56-4ae9-953a-41e42b4ed313
commented
5 years ago Branch pushed to git repo; I updated commit sha1. New commits:
291d584 | Move this reference to the module level. |
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313
commented
5 years ago 7ed8c4ca-6d56-4ae9-953a-41e42b4ed313
commented
5 years ago 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
commented
5 years ago 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
commented
5 years ago 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
commented
5 years ago This looks okay to me, but others should investigate, too.
fchapoton
commented
5 years ago 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".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.rstThis fixes the pyflakes errors:
diff --git a/src/sage_setup/docbuild/ext/sage_autodoc.py b/src/sage_setup/docbuild/ext/sage_autodoc.py
index 9cce254a13..b75791de36 100644
--- a/src/sage_setup/docbuild/ext/sage_autodoc.py
+++ b/src/sage_setup/docbuild/ext/sage_autodoc.py
@@ -30,25 +30,21 @@ AUTHORS:
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.
traceback:
CC: @embray @jdemeyer @kiwifb
Component: python3
Author: Erik Bray, Kwankyu Lee
Branch/Commit:
562ff1eReviewer: Frédéric Chapoton, John Palmieri
Issue created by migration from https://trac.sagemath.org/ticket/27692