py3 failures in sage/misc/sageinspect.py and sagedoc.py

[jhpalmieri]

The Python 3 doctest failures in sage/misc/sageinspect.py and sage/misc/sagedoc.py all arise because of the second line here (from sageinspect.py):

        spec = sage_getargspec(obj)
        s = str(inspect.formatargspec(*spec))

The problem is that inspect.formatargspec is deprecated in Python 3.5, so it prints warning messages. From Sage's point of view, this is annoying, because there is no direct replacement (as far as I can tell) for inspect.formatargspec. The documentation says to use signature and Signature, but we have already obtained the signature information using sage_getargspec and just want to format it. Also, inspect.signature doesn't seem to work on Cython functions — see cython issue 1864, or for a Sage-specific example:

sage: import inspect
sage: from sage.misc.sageinspect import sage_getargspec
sage: from sage.rings.integer import int_to_Z
sage: sage_getargspec(int_to_Z)
ArgSpec(args=['self', 'x'], varargs='args', keywords='kwds', defaults=None)

At this point, with Python 2:

sage: inspect.getargspec(long_to_Z)
...
TypeError: <type 'sage.rings.integer.long_to_Z'> is not a Python function

With Python 3:

sage: inspect.signature(int_to_Z)
...
ValueError: no signature found for builtin type <class 'sage.rings.integer.int_to_Z'>

Fixing this may also fix #27578.

Component: python3

Author: John Palmieri

Branch/Commit: bbd5b28

Reviewer: Vincent Klein

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

[jhpalmieri]

comment:1

This silences the warnings:

diff --git a/src/sage/misc/sageinspect.py b/src/sage/misc/sageinspect.py
index b7571b44f1..e95220ca9a 100644
--- a/src/sage/misc/sageinspect.py
+++ b/src/sage/misc/sageinspect.py
@@ -125,6 +125,7 @@ import os
 import sys
 import tokenize
 import re
+import warnings
 EMBEDDED_MODE = False
 from sage.env import SAGE_LIB
 
@@ -1693,7 +1694,12 @@ def sage_getdef(obj, obj_name=''):
     """
     try:
         spec = sage_getargspec(obj)
-        s = str(inspect.formatargspec(*spec))
+        if six.PY3:
+            with warnings.catch_warnings():
+                warnings.simplefilter("ignore")
+                s = str(inspect.formatargspec(*spec))
+        else:
+            s = str(inspect.formatargspec(*spec))
         s = s.strip('(').strip(')').strip()
         if s[:4] == 'self':
             s = s[4:]

[jhpalmieri]

comment:2

Reimplementing formatargspec (needs doctests ...):

diff --git a/src/sage/misc/sageinspect.py b/src/sage/misc/sageinspect.py
index b7571b44f1..e46b9866ed 100644
--- a/src/sage/misc/sageinspect.py
+++ b/src/sage/misc/sageinspect.py
@@ -1658,6 +1658,49 @@ def sage_getargspec(obj):
         defaults = None
     return inspect.ArgSpec(args, varargs, varkw, defaults)
 
+def joinseq(seq):
+    if len(seq) == 1:
+        return '(' + seq[0] + ',)'
+    else:
+        return '(' + ', '.join(seq) + ')'
+
+def strseq(object, convert, join=joinseq):
+    """
+    Recursively walk a sequence, stringifying each element.
+    """
+    if type(object) in (list, tuple):
+        return join(map(lambda o, c=convert, j=join: strseq(o, c, j), object))
+    else:
+        return convert(object)
+
+def formatargspec(args, varargs=None, varkw=None, defaults=None,
+                  formatarg=str,
+                  formatvarargs=lambda name: '*' + name,
+                  formatvarkw=lambda name: '**' + name,
+                  formatvalue=lambda value: '=' + repr(value),
+                  join=joinseq):
+    """
+    Format an argument spec from the 4 values returned by getargspec.
+
+    The first four arguments are (args, varargs, varkw, defaults).  The
+    other four arguments are the corresponding optional formatting functions
+    that are called to turn names and values into strings.  The ninth
+    argument is an optional function to format the sequence of arguments.
+    """
+    specs = []
+    if defaults:
+        firstdefault = len(args) - len(defaults)
+    for i, arg in enumerate(args):
+        spec = strseq(arg, formatarg, join)
+        if defaults and i >= firstdefault:
+            spec = spec + formatvalue(defaults[i - firstdefault])
+        specs.append(spec)
+    if varargs is not None:
+        specs.append(formatvarargs(varargs))
+    if varkw is not None:
+        specs.append(formatvarkw(varkw))
+    return '(' + ', '.join(specs) + ')'
+
 
 def sage_getdef(obj, obj_name=''):
     r"""
@@ -1693,7 +1736,7 @@ def sage_getdef(obj, obj_name=''):
     """
     try:
         spec = sage_getargspec(obj)
-        s = str(inspect.formatargspec(*spec))
+        s = str(formatargspec(*spec))
         s = s.strip('(').strip(')').strip()
         if s[:4] == 'self':
             s = s[4:]

(This is taken essentially verbatim from Python 2.7's inspect.py.)

[vinklein]

comment:4

As sage has it's own getargspec i think reimplementing formatargspec may be the best solution for now. That way sage would not be affected when inspect.formatargspec will be removed.

Note: sage_getargspec use inspect.ArgSpec and i wonder if this class will be removed along with inspect.formatargspec and inspect.getargspec.

Maybe we should define our own ArgSpec too.

[jhpalmieri]

comment:5

I don't see anything indicating that ArgSpec will be removed. In inspect.py, it is defined by a one-liner:

ArgSpec = namedtuple('ArgSpec', 'args varargs keywords defaults')

(well, two lines if you count from collections import namedtuple). So it's trivial for us to implement if we need to.

Or we could switch to using FullArgSpec, defined in inspect.py by

FullArgSpec = namedtuple('FullArgSpec',
    'args, varargs, varkw, defaults, kwonlyargs, kwonlydefaults, annotations')

I don't think there is any urgency for either of these.

[jhpalmieri]

Branch: u/jhpalmieri/formatargspec

[jhpalmieri]

Commit: 56516cf

[jhpalmieri]

Author: John Palmieri

[jhpalmieri]

comment:7

Okay, here is an implementation of formatargspec, taken directly from Python 3.7's inspect module. This fixes the py3 doctests in sagedoc and sageinspect, and it also silences all of the deprecation warnings in docbuilding (both Python 2 and Python 3).

---

New commits:

56516cf

trac 27971: implement our own version of formatargspec

[kiwifb]

comment:8

It's so nice not to have that formatargspec deprecation message from sphinx in my build logs anymore. Because there is so much of it you cannot even spot the interesting warnings you should be looking for like

[algebras ] <unknown>:1533: DeprecationWarning: invalid escape sequence \y
[repl     ] <unknown>:181: DeprecationWarning: invalid escape sequence \)
[repl     ] <unknown>:365: DeprecationWarning: invalid escape sequence \w

and a few others. Really the stuff of a new ticket but before this branch it was literally impossible to notice that stuff.

[jhpalmieri]

comment:9

Yes, I noticed that, too. Definitely for a different ticket — the issues can be solved completely independently.

[jhpalmieri]

comment:10

Let's not worry about the pyflakes warnings for sage_autodoc.py: they are dealt with at #27692.

[jhpalmieri]

comment:11

Ping?

[vinklein]

comment:12

- ``annotation`` -- blah What does blah mean in this context ?

[sagetrac-git]

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

bbd5b28

trac 27971: implement our own version of formatargspec

[sagetrac-git]

Changed commit from 56516cf to bbd5b28

[jhpalmieri]

comment:14

Replying to @vinklein:

- ``annotation`` -- blah What does blah mean in this context ?

I modified this.

[vinklein]

Reviewer: Vincent Klein

[vinklein]

comment:15

Looks good to me.

[vbraun]

Changed branch from u/jhpalmieri/formatargspec to bbd5b28

[embray]

comment:17

Not in Sage 8.8. Let's please to try keep tickets' milestones related to the release in which we actually intend to include them, and in particular the release in which they were actually included, especially when closing tickets.