Cython: embed signatures in docstrings of Sage library code #17847

jdemeyer · 2015-02-24T10:39:40Z

Before:

sage: print GF(5).factored_unit_order.__doc__
File: sage/rings/finite_rings/finite_field_base.pyx (starting at line 729)

        Returns the factorization of ``self.order()-1``, as a 1-element list.

        The format is for compatibility with
        :mod:`~sage.rings.finite_rings.integer_mod_ring`.

        EXAMPLES::

            sage: GF(7^2,'a').factored_unit_order()
            [2^4 * 3]

After:

sage: print GF(5).factored_unit_order.__doc__
FiniteField.factored_unit_order(self)
File: sage/rings/finite_rings/finite_field_base.pyx (starting at line 729)

        Returns the factorization of ``self.order()-1``, as a 1-element list.

        The format is for compatibility with
        :mod:`~sage.rings.finite_rings.integer_mod_ring`.

        EXAMPLES::

            sage: GF(7^2,'a').factored_unit_order()
            [2^4 * 3]

Upstream bug which will be fixed in Cython 0.23:

http://trac.cython.org/ticket/843

Upstream: Fixed upstream, but not in a stable release.

CC: @simon-king-jena

Component: cython

Author: Jeroen Demeyer, Simon King

Branch/Commit: cc9e9f6

Reviewer: Simon King, Jeroen Demeyer

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

The text was updated successfully, but these errors were encountered:

jdemeyer · 2015-02-24T11:03:54Z

Upstream: Reported upstream. No feedback yet.

jdemeyer · 2015-02-24T14:03:03Z

Branch: u/jdemeyer/ticket/17847

jdemeyer · 2015-02-24T14:03:54Z

Changed branch from u/jdemeyer/ticket/17847 to none

jdemeyer · 2015-02-24T14:03:54Z

comment:6

The attached branch mostly works, but there are still issues in src/sage/misc/sageinspect.py

In particular, I need help with navigating this maze:

def _sage_getsourcelines_name_with_dot(object):
    r"""
    [...]
    AUTHOR:

    - Simon King (2011-09)
    """
    # First, split the name:
    if '.' in object.__name__:
        splitted_name = object.__name__.split('.')
    elif hasattr(object,'__qualname__'):
        splitted_name = object.__qualname__.split('.')
    else:
        splitted_name = object.__name__
    path = object.__module__.split('.')+splitted_name[:-1]
    name = splitted_name[-1]
    try:
        M = __import__(path.pop(0))
    except ImportError:
        try:
            B = object.__base__
            if B is None:
                raise AttributeError
        except AttributeError:
            raise IOError("could not get source code")
        return sage_getsourcelines(B)
    # M should just be the top-most module.
    # Hence, normally it is just 'sage'
    try:
        while path:
            M = getattr(M, path.pop(0))
    except AttributeError:
        try:
            B = object.__base__
            if B is None:
                raise AttributeError
        except AttributeError:
            raise IOError("could not get source code")
        return sage_getsourcelines(B)

    lines, base_lineno = sage_getsourcelines(M)
    [...]

jdemeyer · 2015-02-24T14:05:18Z

Branch: u/jdemeyer/ticket/17847

jdemeyer · 2015-02-24T14:12:35Z

New commits:

`6d2b63a`	`Cython: embed signatures in docstrings`

jdemeyer · 2015-02-24T14:12:35Z

Commit: 6d2b63a

simon-king-jena · 2015-02-24T15:21:24Z

comment:9

Replying to @jdemeyer:

The attached branch mostly works, but there are still issues in src/sage/misc/sageinspect.py

In particular, I need help with navigating this maze:
def _sage_getsourcelines_name_with_dot(object):

That's for dynamic classes, hence, it is pure Python, and the embedding in docstrings isn't relevant here.

Details: In the category framework we have all these nested dynamic classes which get a name that is formed by the name of a class they will be attribute of. Hence:

class Modules(Category):
    def __init__(...):
        ...
    class Homset(Homset_generic):
        def __init__(...)

Hence, you have a class Modules and you have an attribute Modules.Homset that itself is a class (nested classes). Now, you will find:

sage: Modules.Homset.__name__
'Modules.Homset'

Hence, you will NOT find the source code of the class by searching the name in the source file. But the name does give you a possibility to find the code, namely by first searching for the source of the class Modules in the source file and then searching for the source of the class Homset in the source of Modules.

It isn't relevant for Cython classes, since there is a metaclass at work (and you can't have metaclass for Cython classes). It could perhaps be in a cython file, though.

jdemeyer · 2015-02-25T07:43:33Z

Dependencies: #17851

sagetrac-git · 2015-02-25T08:10:33Z

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

`98a5813`	`Nuke Cython cache when Cython version changes`
`9cd65e1`	`Merge commit '98a581300ec495b9c39ebe3484f407a491f8bb95' into t/17847/ticket/17847`

sagetrac-git · 2015-02-25T08:10:33Z

Changed commit from 6d2b63a to 9cd65e1

jdemeyer · 2015-02-25T09:23:16Z

comment:13

Replying to @simon-king-jena:

Replying to @jdemeyer:
The attached branch mostly works, but there are still issues in src/sage/misc/sageinspect.py

In particular, I need help with navigating this maze:
def _sage_getsourcelines_name_with_dot(object):
That's for dynamic classes, hence, it is pure Python, and the embedding in docstrings isn't relevant here.

Apparently it is relevant, since I'm getting doctest failures in this function:

sage -t src/sage/misc/nested_class_test.py
**********************************************************************
File "src/sage/misc/nested_class_test.py", line 215, in sage.misc.nested_class_test.TestNestedParent
Failed example:
    print sage_getsource(E)
Exception raised:
    Traceback (most recent call last):
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 488, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 850, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.misc.nested_class_test.TestNestedParent[5]>", line 1, in <module>
        print sage_getsource(E)
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/misc/sageinspect.py", line 1641, in sage_getsource
        t = sage_getsourcelines(obj, is_binary)
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/misc/sageinspect.py", line 1895, in sage_getsourcelines
        return obj._sage_src_lines_()
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/structure/dynamic_class.py", line 392, in _sage_src_lines
        return sage_getsourcelines(doccls)
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/misc/sageinspect.py", line 1921, in sage_getsourcelines
        return _sage_getsourcelines_name_with_dot(obj)
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/misc/sageinspect.py", line 1755, in _sage_getsourcelines_name_with_dot
        raise IOError('could not find class definition')
    IOError: could not find class definition
**********************************************************************

jdemeyer · 2015-02-25T09:25:40Z

comment:14

The TestNestedParent does inherit from the Cython class Parent, perhaps that's how Cython comes in.

jdemeyer · 2015-04-07T08:34:23Z

Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.

jdemeyer · 2015-04-07T08:36:33Z

Changed dependencies from #17851 to none

jdemeyer · 2015-04-07T08:36:33Z

Author: Jeroen Demeyer

sagetrac-git · 2015-04-07T09:41:10Z

Changed commit from 9cd65e1 to 1a36cc9

sagetrac-git · 2015-04-07T09:41:10Z

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

`1a36cc9`	`Cython: embed signatures in docstrings`

jdemeyer · 2015-04-07T09:49:33Z

comment:18

Rebased and included upstream Cython patch, but it still doesn't work due to the issue with _sage_getsourcelines_name_with_dot()

simon-king-jena · 2015-04-07T10:04:44Z

comment:19

OK, I'll have a look.

Do I need to do "sage -ba" to build it, or is that done automatically because of dependency checking?

Best regards,
Simon

jdemeyer · 2015-04-09T13:30:32Z

comment:96

I couldn't find any difference in behaviour between foo? or foo?? with or without this patch.

jdemeyer · 2015-04-09T13:34:06Z

comment:97

What is this for?

getattr(init, 'im_class', None) == typ

sagetrac-git · 2015-04-09T13:42:55Z

Changed commit from 51aa68c to 02ee3cc

sagetrac-git · 2015-04-09T13:42:55Z

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

`02ee3cc`	`Fix raise syntax`

simon-king-jena · 2015-04-09T13:46:38Z

comment:99

Replying to @sagetrac-git:

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

`02ee3cc`	`Fix raise syntax`

Thanks, I thought that except (TypeError, AttributeError), err assigns the caught error to err.

simon-king-jena · 2015-04-09T13:48:55Z

comment:100

Replying to @jdemeyer:

What is this for?

getattr(init, 'im_class', None) == typ

I wanted to test that the init method belongs to typ and is not inherited from a super class.

But on second thought, it would perhaps be better to work with inheritance: After all, docstrings are inherited, too.

simon-king-jena · 2015-04-09T13:51:10Z

comment:101

Replying to @simon-king-jena:

I wanted to test that the init method belongs to typ and is not inherited from a super class.

The original rationale was this: The init method may contain embedding information for the class. If it is obtained from the super-class, then the embedding information belongs to a different class and is thus misleading.

jdemeyer · 2015-04-09T13:52:03Z

comment:102

Replying to @simon-king-jena:

But on second thought, it would perhaps be better to work with inheritance: After all, docstrings are inherited, too.

I don't think this should be changed.
I would not like to see the doc of Parent just because esomething something inherits from Parent.

jdemeyer · 2015-04-09T13:52:44Z

comment:103

For me, this branch is good to go if you add some comment in the sources about this part:

            if (getattr(init, '__objclass__', None) or
                getattr(init, 'im_class', None)) == typ:
                return sage_getdoc_original(init)

simon-king-jena · 2015-04-09T13:54:18Z

comment:104

Replying to @jdemeyer:

For me, this branch is good to go if you add some comment in the sources about this part:
            if (getattr(init, '__objclass__', None) or
                getattr(init, 'im_class', None)) == typ:
                return sage_getdoc_original(init)

I will do so a bit later, as I have an appointment now.

jdemeyer · 2015-04-09T14:34:50Z

comment:105

Hmm, this patch doesn't behave so nicely for some of Python's docs:

sage: print type.__doc__
type(object) -> the object's type
type(name, bases, dict) -> a new type

sage: print sage_getdoc_original(type)
type(name, bases, dict) -> a new type

This causes

sage -t --long src/sage/repl/ipython_tests.py
**********************************************************************
File "src/sage/repl/ipython_tests.py", line 10, in sage.repl.ipython_tests
Failed example:
    shell.run_cell(u'%pinfo dummy')
Expected:
    Type:            function
    String form:     <function dummy at 0x...>
    File:            /.../sage/repl/ipython_tests.py
    Definition:      dummy(argument, optional=None)
    Docstring:
       Dummy Docstring Title
    <BLANKLINE>
       Dummy docstring explanation.
    <BLANKLINE>
       INPUT:
    <BLANKLINE>
       * "argument" -- anything. Dummy argument.
    <BLANKLINE>
       * "optional" -- anything (optional). Dummy optional.
    <BLANKLINE>
       EXAMPLES:
    ...
    Class docstring:
    function(code, globals[, name[, argdefs[, closure]]])
    <BLANKLINE>
    Create a function object from a code object and a dictionary. The
    optional name string overrides the name from the code object. The
    optional argdefs tuple specifies the default argument values. The
    optional closure tuple supplies the bindings for free variables.
    Init docstring:  x.__init__(...) initializes x; see help(type(x)) for signature
Got:
    Type:            function
    String form:     <function dummy at 0x7f8a0ffe76e0>
    File:            /usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/repl/ipython_tests.py
    Definition:      dummy(argument, optional=None)
    Docstring:
       Dummy Docstring Title
    <BLANKLINE>
       Dummy docstring explanation.
    <BLANKLINE>
       INPUT:
    <BLANKLINE>
       * "argument" -- anything. Dummy argument.
    <BLANKLINE>
       * "optional" -- anything (optional). Dummy optional.
    <BLANKLINE>
       EXAMPLES:
    <BLANKLINE>
          sage: from sage.repl.ipython_tests import dummy sage: dummy(1)
          'Source code would be here'
    <BLANKLINE>
    Class docstring:
    Create a function object from a code object and a dictionary. The
    optional name string overrides the name from the code object. The
    optional argdefs tuple specifies the default argument values. The
    optional closure tuple supplies the bindings for free variables.
    Init docstring:  x.__init__(...) initializes x; see help(type(x)) for signature
**********************************************************************

simon-king-jena · 2015-04-09T15:53:44Z

comment:106

Replying to @jdemeyer:

Hmm, this patch doesn't behave so nicely for some of Python's docs:
sage: print sage_getdoc_original(type)
type(name, bases, dict) -> a new type

Yes. That's because of my attempt to remove the signature. After all, what I have is only a heuristics.

How could we improve the heuristics? Special-case type? Test whether the first line ends with ')'?

I'll finish a commit now. Please wait.

simon-king-jena · 2015-04-09T15:59:34Z

Changed branch from u/jdemeyer/ticket/17847 to u/SimonKing/ticket/17847

simon-king-jena · 2015-04-09T16:01:03Z

Changed branch from u/SimonKing/ticket/17847 to u/jdemeyer/ticket/17847

simon-king-jena · 2015-04-09T16:01:41Z

Changed branch from u/jdemeyer/ticket/17847 to u/SimonKing/ticket/17847

simon-king-jena · 2015-04-09T16:01:41Z

comment:109

Why has the branch that I just pushed be reverted???

New commits:

`934490a`	`Add a comment; improve heuristics to strip the embedded signature`

simon-king-jena · 2015-04-09T16:01:41Z

Changed commit from 02ee3cc to 934490a

simon-king-jena · 2015-04-09T16:02:13Z

comment:110

Now it should be ready for review.

jdemeyer · 2015-04-10T08:27:56Z

Changed branch from u/SimonKing/ticket/17847 to u/jdemeyer/ticket/17847

simon-king-jena · 2015-04-10T09:28:18Z

comment:112

Sorry that I didn't fix that doctest.

New commits:

`cc9e9f6`	`Fix IPython doctest`

simon-king-jena · 2015-04-10T09:28:18Z

Changed commit from 934490a to cc9e9f6

vbraun · 2015-04-14T19:43:56Z

Changed branch from u/jdemeyer/ticket/17847 to cc9e9f6

jdemeyer added this to the sage-6.6 milestone Feb 24, 2015

jdemeyer added c: cython labels Feb 24, 2015

This comment has been minimized.

Sign in to view

jdemeyer added s: needs work and removed s: needs review labels Apr 9, 2015

simon-king-jena added s: needs review and removed s: needs work labels Apr 9, 2015

jdemeyer added s: positive review and removed s: needs review labels Apr 10, 2015

vbraun removed the s: positive review label Apr 14, 2015

vbraun closed this as completed in 596567e Apr 14, 2015

Cython: embed signatures in docstrings of Sage library code #17847

Cython: embed signatures in docstrings of Sage library code #17847

Comments

jdemeyer commented Feb 24, 2015

This comment has been minimized.

This comment has been minimized.

jdemeyer commented Feb 24, 2015

This comment has been minimized.

This comment has been minimized.

jdemeyer commented Feb 24, 2015

jdemeyer commented Feb 24, 2015

jdemeyer commented Feb 24, 2015

jdemeyer commented Feb 24, 2015

jdemeyer commented Feb 24, 2015

jdemeyer commented Feb 24, 2015

simon-king-jena commented Feb 24, 2015

jdemeyer commented Feb 25, 2015

This comment has been minimized.

sagetrac-git mannequin commented Feb 25, 2015

sagetrac-git mannequin commented Feb 25, 2015

jdemeyer commented Feb 25, 2015

jdemeyer commented Feb 25, 2015

This comment has been minimized.

jdemeyer commented Apr 7, 2015

jdemeyer commented Apr 7, 2015

jdemeyer commented Apr 7, 2015

sagetrac-git mannequin commented Apr 7, 2015

sagetrac-git mannequin commented Apr 7, 2015

jdemeyer commented Apr 7, 2015

simon-king-jena commented Apr 7, 2015

jdemeyer commented Apr 9, 2015

jdemeyer commented Apr 9, 2015

sagetrac-git mannequin commented Apr 9, 2015

sagetrac-git mannequin commented Apr 9, 2015

simon-king-jena commented Apr 9, 2015

simon-king-jena commented Apr 9, 2015

simon-king-jena commented Apr 9, 2015

jdemeyer commented Apr 9, 2015

jdemeyer commented Apr 9, 2015

simon-king-jena commented Apr 9, 2015

jdemeyer commented Apr 9, 2015

simon-king-jena commented Apr 9, 2015

simon-king-jena commented Apr 9, 2015

simon-king-jena commented Apr 9, 2015

simon-king-jena commented Apr 9, 2015

simon-king-jena commented Apr 9, 2015

simon-king-jena commented Apr 9, 2015

simon-king-jena commented Apr 9, 2015

jdemeyer commented Apr 10, 2015

simon-king-jena commented Apr 10, 2015

simon-king-jena commented Apr 10, 2015

vbraun commented Apr 14, 2015