#25840 closed enhancement (fixed)

py3: towards pdf-docbuild. some care for automethod

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.4
Component: python3 Keywords:
Cc: tscrim, jdemeyer Merged in:
Authors: Frédéric Chapoton Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: acad1de (Commits) Commit: acad1de09068e2b664e35f5ac1f912bda64a813a
Dependencies: Stopgaps:

Description


Change History (20)

comment:1 Changed 21 months ago by chapoton

  • Branch set to u/chapoton/25840
  • Commit set to efc23b64dc8dcd038d7fc6973a8d6a5a6d1c0faf
  • Status changed from new to needs_review

New commits:

efc23b6py3: fix use of automethod for pdf docbuild

comment:2 Changed 21 months ago by chapoton

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

comment:3 Changed 21 months ago by chapoton

  • Cc tscrim jdemeyer added

comment:4 Changed 21 months ago by chapoton

any opinion on this one ?

comment:5 Changed 20 months ago by jhpalmieri

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:

  • src/sage_setup/docbuild/ext/multidocs.py

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

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.

comment:6 Changed 20 months ago by jhpalmieri

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

comment:7 Changed 20 months ago by jhpalmieri

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

  • src/doc/en/reference/notebook/index.rst

    diff --git a/src/doc/en/reference/notebook/index.rst b/src/doc/en/reference/notebook/index.rst
    index 144b8c20e0..9196a59cdd 100644
    a b default notebook to the `Jupyter notebook 
    99export service.)
    1010
    1111
    12 Legacy Sage Notebook (sagenb)
    13 -----------------------------
    14 
    15 .. toctree::
    16    :maxdepth: 2
    17 
    18    sagenb/notebook/notebook_object
    19    sagenb/notebook/config
    20    sagenb/notebook/interact
    21    sagenb/notebook/cell
    22    sagenb/notebook/worksheet
    23    sagenb/notebook/notebook
    24    sagenb/notebook/js
    25    sagenb/notebook/css
    26    sagenb/notebook/template
    27 
    28 Other Servers
    29 -------------
    30 
    31 .. toctree::
    32    :maxdepth: 2
    33 
    34    sagenb/notebook/challenge
    35 
    36 Miscellaneous
    37 -------------
    38 
    39 .. toctree::
    40    :maxdepth: 2
    41 
    42    sagenb/misc/misc
    43    sagenb/misc/support
    44    sagenb/misc/introspect
    45    sagenb/misc/sageinspect
    46    sagenb/misc/sphinxify
    47    sagenb/notebook/docHTMLProcessor
    48 
    49 Storage
    50 -------
    51 
    52 .. toctree::
    53    :maxdepth: 2
    54 
    55    sagenb/storage/abstract_storage
    56    sagenb/storage/filesystem_storage
    57 
    58 .. Commented out, for now.
    59 
    60    Interfaces
    61    ----------
    62 
    63    SKIP sagenb/interfaces/worksheet_process
    64    SKIP sagenb/interfaces/reference
    65    SKIP sagenb/interfaces/expect
    66 
    6712.. include:: ../footer.txt
  • src/sage/geometry/polyhedron/backend_cdd.py

    diff --git a/src/sage/geometry/polyhedron/backend_cdd.py b/src/sage/geometry/polyhedron/backend_cdd.py
    index b4672e8f1b..9544cf197c 100644
    a b class Polyhedron_cdd(Polyhedron_base): 
    218218        cddout = cddout.splitlines()
    219219
    220220        def parse_indices(count, cdd_indices, cdd_indices_to_sage_indices=None):
    221             cdd_indices = map(int, cdd_indices)
     221            cdd_indices = list(map(int, cdd_indices))
    222222            if cdd_indices_to_sage_indices is None:
    223223                cdd_indices_to_sage_indices = {i:i-1 for i in cdd_indices}
    224224            if count < 0:
    class Polyhedron_cdd(Polyhedron_base): 
    247247            assert self.ambient_dim() == dimension - 1, "Unexpected ambient dimension"
    248248            assert len(data) == count, "Unexpected number of lines"
    249249            for i, line in enumerate(data):
    250                 coefficients = map(self.base_ring(), line)
     250                coefficients = list(map(self.base_ring(), line))
    251251                if coefficients[0] != 0 and all([e == 0 for e in coefficients[1:]]):
    252252                    # cddlib sometimes includes an implicit plane at infinity: 1 0 0 ... 0
    253253                    # We do not care about this entry.
  • src/sage/geometry/polyhedron/representation.py

    diff --git a/src/sage/geometry/polyhedron/representation.py b/src/sage/geometry/polyhedron/representation.py
    index 699c56e066..e53fee0921 100644
    a b class Vrepresentation(PolyhedronRepresentation): 
    920920            sage: TestSuite(pV).run(skip='_test_pickling')
    921921        """
    922922        assert polyhedron.parent() is self._polyhedron_parent
     923        data = list(data)
    923924        if len(data) != self._vector.degree():
    924925            raise ValueError('V-representation data requires a list of length ambient_dim')
    925926
  • src/sage/numerical/backends/logging_backend.py

    diff --git a/src/sage/numerical/backends/logging_backend.py b/src/sage/numerical/backends/logging_backend.py
    index b2c56e9a6c..79a7b60610 100644
    a b def _override_attr(attr): 
    205205        sage: from sage.numerical.backends.logging_backend import _override_attr
    206206    """
    207207    a = getattr(LoggingBackend, attr)
    208     if callable(a):
     208    if False and callable(a):
    209209        # make an unbound method
    210210        import types
    211211        _mm = types.MethodType(_make_wrapper(GenericBackend(), attr), None, LoggingBackend)
  • src/sage/sets/cartesian_product.py

    diff --git a/src/sage/sets/cartesian_product.py b/src/sage/sets/cartesian_product.py
    index c84c44747b..f3354107fb 100644
    a b class CartesianProduct(UniqueRepresentation, Parent): 
    4646            and Category of Cartesian products of monoids
    4747            and Category of Cartesian products of finite enumerated sets
    4848
    49     .. automethod:: _cartesian_product_of_elements
     49    .. automethod:: CartesianProduct._cartesian_product_of_elements
    5050    """
    5151    def __init__(self, sets, category, flatten=False):
    5252        r"""
  • src/sage/structure/element.pyx

    diff --git a/src/sage/structure/element.pyx b/src/sage/structure/element.pyx
    index f888c4f57a..989a04c8f0 100644
    a b cdef class Element(SageObject): 
    380380    .. automethod:: __sub__
    381381    .. automethod:: __neg__
    382382    .. automethod:: __mul__
    383     .. automethod:: Element.__div__
    384383    .. automethod:: __truediv__
    385384    .. automethod:: __floordiv__
    386385    .. automethod:: __mod__
  • src/sage_setup/docbuild/ext/multidocs.py

    diff --git a/src/sage_setup/docbuild/ext/multidocs.py b/src/sage_setup/docbuild/ext/multidocs.py
    index 5dec201010..62d39461df 100644
    a b def write_citations(app, citations): 
    236236    """
    237237    from sage.misc.temporary_file import atomic_write
    238238    outdir = citation_dir(app)
    239     with atomic_write(os.path.join(outdir, CITE_FILENAME)) as f:
     239    with atomic_write(os.path.join(outdir, CITE_FILENAME), binary=True) as f:
    240240        cPickle.dump(citations, f)
    241241    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)
Last edited 20 months ago by jhpalmieri (previous) (diff)

comment:8 Changed 20 months ago by chapoton

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

comment:9 Changed 20 months ago by git

  • Commit changed from efc23b64dc8dcd038d7fc6973a8d6a5a6d1c0faf to 40655f3f042587df916919f152a530796a7f38f1

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

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

comment:10 follow-up: Changed 20 months ago by chapoton

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.

comment:11 in reply to: ↑ 10 Changed 20 months ago by jhpalmieri

Replying to chapoton:

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.

comment:12 Changed 20 months ago by jhpalmieri

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.

comment:13 Changed 20 months ago by git

  • Commit changed from 40655f3f042587df916919f152a530796a7f38f1 to acad1de09068e2b664e35f5ac1f912bda64a813a

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

acad1depy3: fix use of automethod for pdf docbuild

comment:14 Changed 20 months ago by chapoton

I have undone the change to {{{div}} automethod. So this should be ready to go.

Version 0, edited 20 months ago by chapoton (next)

comment:15 Changed 20 months ago by chapoton

  • Milestone changed from sage-8.3 to sage-8.4

comment:16 Changed 20 months ago by chapoton

  • Reviewers set to John Palmieri

comment:17 follow-up: Changed 20 months ago by jhpalmieri

  • Status changed from needs_review to positive_review

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.

comment:18 Changed 20 months ago by jdemeyer

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.

comment:19 in reply to: ↑ 17 Changed 20 months ago by jdemeyer

Replying to jhpalmieri:

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

I created #26043 for that.

comment:20 Changed 20 months ago by vbraun

  • Branch changed from u/chapoton/25840 to acad1de09068e2b664e35f5ac1f912bda64a813a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.