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

py3: towards pdf-docbuild. some care for automethod #25840

Closed
fchapoton opened this issue Jul 12, 2018 · 22 comments
Closed

py3: towards pdf-docbuild. some care for automethod #25840

fchapoton opened this issue Jul 12, 2018 · 22 comments

Comments

@fchapoton
Copy link
Contributor

CC: @tscrim @jdemeyer

Component: python3

Author: Frédéric Chapoton

Branch/Commit: acad1de

Reviewer: John Palmieri

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

@fchapoton fchapoton added this to the sage-8.3 milestone Jul 12, 2018
@fchapoton
Copy link
Contributor Author

Commit: efc23b6

@fchapoton
Copy link
Contributor Author

New commits:

efc23b6py3: fix use of automethod for pdf docbuild

@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/25840

@fchapoton
Copy link
Contributor Author

comment:2

Not sure what to do. Is this a correct fix, or is this just curing the symptoms and not the disease ?

@fchapoton
Copy link
Contributor Author

comment:4

any opinion on this one ?

@jhpalmieri
Copy link
Member

comment:5

I don't have any objections to this approach, and since no one else has spoken up, I say we go ahead. Two comments: first, I get an error with docbuilding (html, not pdf) with or without your change in about __div__ in structure.pyx. Second, I also suggest this change:

diff --git a/src/sage_setup/docbuild/ext/multidocs.py b/src/sage_setup/docbuild/ext/multidocs.py
index 5dec201010..62d39461df 100644
--- a/src/sage_setup/docbuild/ext/multidocs.py
+++ b/src/sage_setup/docbuild/ext/multidocs.py
@@ -236,7 +236,7 @@ def write_citations(app, citations):
     """
     from sage.misc.temporary_file import atomic_write
     outdir = citation_dir(app)
-    with atomic_write(os.path.join(outdir, CITE_FILENAME)) as f:
+    with atomic_write(os.path.join(outdir, CITE_FILENAME), binary=True) as f:
         cPickle.dump(citations, f)
     app.info("Saved pickle file: %s" % CITE_FILENAME)
 

atomic_write has binary=True as default for Python 2 but not for Python 3, but we are writing binary data here regardless of the version of Python.

@jhpalmieri
Copy link
Member

comment:6

Also regarding the docs, both html and pdf, I'm getting some weird error message about numerical/backends/logging_backend.py. I don't know a good way to fix that.

[docpdf]   File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/PYTHON3/sage-8.4.beta0/local/lib/python3.6/site-packages/sage/numerical/backends/logging_backend.py", line 211, in _override_attr
[docpdf]     _mm = types.MethodType(_make_wrapper(GenericBackend(), attr), None, LoggingBackend)
[docpdf] TypeError: method expected 2 arguments, got 3

@jhpalmieri
Copy link
Member

comment:7

I can get the PDF docs to build with the following changes, at least some of which are the wrong thing to do:

diff --git a/src/doc/en/reference/notebook/index.rst b/src/doc/en/reference/notebook/index.rst
index 144b8c20e0..9196a59cdd 100644
--- a/src/doc/en/reference/notebook/index.rst
+++ b/src/doc/en/reference/notebook/index.rst
@@ -9,59 +9,4 @@ default notebook to the `Jupyter notebook
 export service.)
 
 
-Legacy Sage Notebook (sagenb)
------------------------------
-
-.. toctree::
-   :maxdepth: 2
-
-   sagenb/notebook/notebook_object
-   sagenb/notebook/config
-   sagenb/notebook/interact
-   sagenb/notebook/cell
-   sagenb/notebook/worksheet
-   sagenb/notebook/notebook
-   sagenb/notebook/js
-   sagenb/notebook/css
-   sagenb/notebook/template
-
-Other Servers
--------------
-
-.. toctree::
-   :maxdepth: 2
-
-   sagenb/notebook/challenge
-
-Miscellaneous
--------------
-
-.. toctree::
-   :maxdepth: 2
-
-   sagenb/misc/misc
-   sagenb/misc/support
-   sagenb/misc/introspect
-   sagenb/misc/sageinspect
-   sagenb/misc/sphinxify
-   sagenb/notebook/docHTMLProcessor
-
-Storage
--------
-
-.. toctree::
-   :maxdepth: 2
-
-   sagenb/storage/abstract_storage
-   sagenb/storage/filesystem_storage
-
-.. Commented out, for now.
-
-   Interfaces
-   ----------
-
-   SKIP sagenb/interfaces/worksheet_process
-   SKIP sagenb/interfaces/reference
-   SKIP sagenb/interfaces/expect
-
 .. include:: ../footer.txt
diff --git a/src/sage/geometry/polyhedron/backend_cdd.py b/src/sage/geometry/polyhedron/backend_cdd.py
index b4672e8f1b..9544cf197c 100644
--- a/src/sage/geometry/polyhedron/backend_cdd.py
+++ b/src/sage/geometry/polyhedron/backend_cdd.py
@@ -218,7 +218,7 @@ class Polyhedron_cdd(Polyhedron_base):
         cddout = cddout.splitlines()
 
         def parse_indices(count, cdd_indices, cdd_indices_to_sage_indices=None):
-            cdd_indices = map(int, cdd_indices)
+            cdd_indices = list(map(int, cdd_indices))
             if cdd_indices_to_sage_indices is None:
                 cdd_indices_to_sage_indices = {i:i-1 for i in cdd_indices}
             if count < 0:
@@ -247,7 +247,7 @@ class Polyhedron_cdd(Polyhedron_base):
             assert self.ambient_dim() == dimension - 1, "Unexpected ambient dimension"
             assert len(data) == count, "Unexpected number of lines"
             for i, line in enumerate(data):
-                coefficients = map(self.base_ring(), line)
+                coefficients = list(map(self.base_ring(), line))
                 if coefficients[0] != 0 and all([e == 0 for e in coefficients[1:]]):
                     # cddlib sometimes includes an implicit plane at infinity: 1 0 0 ... 0
                     # We do not care about this entry.
diff --git a/src/sage/geometry/polyhedron/representation.py b/src/sage/geometry/polyhedron/representation.py
index 699c56e066..e53fee0921 100644
--- a/src/sage/geometry/polyhedron/representation.py
+++ b/src/sage/geometry/polyhedron/representation.py
@@ -920,6 +920,7 @@ class Vrepresentation(PolyhedronRepresentation):
             sage: TestSuite(pV).run(skip='_test_pickling')
         """
         assert polyhedron.parent() is self._polyhedron_parent
+        data = list(data)
         if len(data) != self._vector.degree():
             raise ValueError('V-representation data requires a list of length ambient_dim')
 
diff --git a/src/sage/numerical/backends/logging_backend.py b/src/sage/numerical/backends/logging_backend.py
index b2c56e9a6c..79a7b60610 100644
--- a/src/sage/numerical/backends/logging_backend.py
+++ b/src/sage/numerical/backends/logging_backend.py
@@ -205,7 +205,7 @@ def _override_attr(attr):
         sage: from sage.numerical.backends.logging_backend import _override_attr
     """
     a = getattr(LoggingBackend, attr)
-    if callable(a):
+    if False and callable(a):
         # make an unbound method
         import types
         _mm = types.MethodType(_make_wrapper(GenericBackend(), attr), None, LoggingBackend)
diff --git a/src/sage/sets/cartesian_product.py b/src/sage/sets/cartesian_product.py
index c84c44747b..f3354107fb 100644
--- a/src/sage/sets/cartesian_product.py
+++ b/src/sage/sets/cartesian_product.py
@@ -46,7 +46,7 @@ class CartesianProduct(UniqueRepresentation, Parent):
             and Category of Cartesian products of monoids
             and Category of Cartesian products of finite enumerated sets
 
-    .. automethod:: _cartesian_product_of_elements
+    .. automethod:: CartesianProduct._cartesian_product_of_elements
     """
     def __init__(self, sets, category, flatten=False):
         r"""
diff --git a/src/sage/structure/element.pyx b/src/sage/structure/element.pyx
index f888c4f57a..989a04c8f0 100644
--- a/src/sage/structure/element.pyx
+++ b/src/sage/structure/element.pyx
@@ -380,7 +380,6 @@ cdef class Element(SageObject):
     .. automethod:: __sub__
     .. automethod:: __neg__
     .. automethod:: __mul__
-    .. automethod:: Element.__div__
     .. automethod:: __truediv__
     .. automethod:: __floordiv__
     .. automethod:: __mod__
diff --git a/src/sage_setup/docbuild/ext/multidocs.py b/src/sage_setup/docbuild/ext/multidocs.py
index 5dec201010..62d39461df 100644
--- a/src/sage_setup/docbuild/ext/multidocs.py
+++ b/src/sage_setup/docbuild/ext/multidocs.py
@@ -236,7 +236,7 @@ def write_citations(app, citations):
     """
     from sage.misc.temporary_file import atomic_write
     outdir = citation_dir(app)
-    with atomic_write(os.path.join(outdir, CITE_FILENAME)) as f:
+    with atomic_write(os.path.join(outdir, CITE_FILENAME), binary=True) as f:
         cPickle.dump(citations, f)
     app.info("Saved pickle file: %s" % CITE_FILENAME)

The first one removes the sagenb pages from the reference manual. The second and third deal with map objects. The fifth is similar to others on this ticket. The seventh is the one I mentioned in comment:5. The fourth (logging_backend.py) and sixth (element.pyx) are the wrong things to do, but I didn't know how to fix those problems.

The html docs don't build even with these changes:

[dochtml] OSError: [categorie] /Users/palmieri/Desktop/Sage_stuff/sage_builds/PYTHON3/sage-8.4.beta0/src/doc/en/reference/categories/sage/categories/facade_sets.rst:4: WARNING: undefined label: facade-sets (if the link has no caption the label must precede a section header)

@fchapoton
Copy link
Contributor Author

comment:8

I have split up a ticket with the changes in polyhedron/ : #26035

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 9, 2018

Changed commit from efc23b6 to 40655f3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 9, 2018

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

788e460Merge branch 'u/chapoton/25840' in 8.4.b0
40655f3more automethod fixes

@fchapoton
Copy link
Contributor Author

comment:10

For the sagenb problem, maybe we can just remove the Miscellanous part of the doc. but in another ticket.

The __div__ problem is quite mysterious to me.

@jhpalmieri
Copy link
Member

comment:11

Replying to @fchapoton:

For the sagenb problem, maybe we can just remove the Miscellanous part of the doc. but in another ticket.

Sounds good.

The __div__ problem is quite mysterious to me.

Right, me too.

@jhpalmieri
Copy link
Member

comment:12

Should we accept this as is, perhaps without the change to __div__ (since I don't think it helps)? It is certainly an improvement, even though it doesn't fix everything.

Regarding __div__, could it be a Cython problem, perhaps to do with inheritance? I ran into something similar a few days ago when I was playing with the matrix directory and Python 3. In the file matrix/matrix_integer_dense.pyx, in the class Matrix_integer_dense, there is a method __nonzero__, but when you define a matrix in this class, it doesn't have this method:

sage: a = MatrixSpace(ZZ, 2, 3)(range(6))
sage: type(a)
<class 'sage.matrix.matrix_integer_dense.Matrix_integer_dense'>
sage: a.__nonzero__()
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-9-74dab0bd3f82> in <module>()
----> 1 a.__nonzero__()

    [snip]

AttributeError: 'sage.matrix.matrix_integer_dense.Matrix_integer_dense' object has no attribute '__nonzero__'

If I change the name of the method to anything else, e.g., __nonzero2__, then a.__nonzero2__() works.

In any case, the __div__ problem may be complicated, so I would prefer to deal with it on a separate ticket.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 10, 2018

Changed commit from 40655f3 to acad1de

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 10, 2018

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

acad1depy3: fix use of automethod for pdf docbuild

@fchapoton
Copy link
Contributor Author

comment:14

I have undone the change to __div__ automethod. So this should be ready to go.

@fchapoton fchapoton modified the milestones: sage-8.3, sage-8.4 Aug 10, 2018
@fchapoton
Copy link
Contributor Author

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:17

Okay, I'm happy with this. For other tickets: the __div__ issue, plus I don't know what to do with src/sage/numerical/backends/logging_backend.py, mainly because I don't understand what that code is doing.

@jdemeyer
Copy link

comment:18

Isn't the problem with __div__ simply that __div__ does not exist on Python 3? Although that does not explain why Element.__div__ does work.

@jdemeyer
Copy link

comment:19

Replying to @jhpalmieri:

I don't know what to do with src/sage/numerical/backends/logging_backend.py

I created #26043 for that.

@vbraun
Copy link
Member

vbraun commented Aug 17, 2018

Changed branch from u/chapoton/25840 to acad1de

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

4 participants