Opened 5 months ago

Closed 5 months ago

Last modified 4 months ago

#27971 closed defect (fixed)

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

Reported by: jhpalmieri Owned by:
Priority: major Milestone: sage-8.9
Component: python3 Keywords:
Cc: Merged in:
Authors: John Palmieri Reviewers: Vincent Klein
Report Upstream: N/A Work issues:
Branch: bbd5b28 (Commits) Commit: bbd5b28fc8e451bea4f87c2f2b0c27586386262a
Dependencies: Stopgaps:

Description (last modified by 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.

Change History (17)

comment:1 Changed 5 months ago by jhpalmieri

This silences the warnings:

  • src/sage/misc/sageinspect.py

    diff --git a/src/sage/misc/sageinspect.py b/src/sage/misc/sageinspect.py
    index b7571b44f1..e95220ca9a 100644
    a b import os 
    125125import sys
    126126import tokenize
    127127import re
     128import warnings
    128129EMBEDDED_MODE = False
    129130from sage.env import SAGE_LIB
    130131
    def sage_getdef(obj, obj_name=''): 
    16931694    """
    16941695    try:
    16951696        spec = sage_getargspec(obj)
    1696         s = str(inspect.formatargspec(*spec))
     1697        if six.PY3:
     1698            with warnings.catch_warnings():
     1699                warnings.simplefilter("ignore")
     1700                s = str(inspect.formatargspec(*spec))
     1701        else:
     1702            s = str(inspect.formatargspec(*spec))
    16971703        s = s.strip('(').strip(')').strip()
    16981704        if s[:4] == 'self':
    16991705            s = s[4:]

comment:2 Changed 5 months ago by jhpalmieri

Reimplementing formatargspec (needs doctests ...):

  • src/sage/misc/sageinspect.py

    diff --git a/src/sage/misc/sageinspect.py b/src/sage/misc/sageinspect.py
    index b7571b44f1..e46b9866ed 100644
    a b def sage_getargspec(obj): 
    16581658        defaults = None
    16591659    return inspect.ArgSpec(args, varargs, varkw, defaults)
    16601660
     1661def joinseq(seq):
     1662    if len(seq) == 1:
     1663        return '(' + seq[0] + ',)'
     1664    else:
     1665        return '(' + ', '.join(seq) + ')'
     1666
     1667def strseq(object, convert, join=joinseq):
     1668    """
     1669    Recursively walk a sequence, stringifying each element.
     1670    """
     1671    if type(object) in (list, tuple):
     1672        return join(map(lambda o, c=convert, j=join: strseq(o, c, j), object))
     1673    else:
     1674        return convert(object)
     1675
     1676def formatargspec(args, varargs=None, varkw=None, defaults=None,
     1677                  formatarg=str,
     1678                  formatvarargs=lambda name: '*' + name,
     1679                  formatvarkw=lambda name: '**' + name,
     1680                  formatvalue=lambda value: '=' + repr(value),
     1681                  join=joinseq):
     1682    """
     1683    Format an argument spec from the 4 values returned by getargspec.
     1684
     1685    The first four arguments are (args, varargs, varkw, defaults).  The
     1686    other four arguments are the corresponding optional formatting functions
     1687    that are called to turn names and values into strings.  The ninth
     1688    argument is an optional function to format the sequence of arguments.
     1689    """
     1690    specs = []
     1691    if defaults:
     1692        firstdefault = len(args) - len(defaults)
     1693    for i, arg in enumerate(args):
     1694        spec = strseq(arg, formatarg, join)
     1695        if defaults and i >= firstdefault:
     1696            spec = spec + formatvalue(defaults[i - firstdefault])
     1697        specs.append(spec)
     1698    if varargs is not None:
     1699        specs.append(formatvarargs(varargs))
     1700    if varkw is not None:
     1701        specs.append(formatvarkw(varkw))
     1702    return '(' + ', '.join(specs) + ')'
     1703
    16611704
    16621705def sage_getdef(obj, obj_name=''):
    16631706    r"""
    def sage_getdef(obj, obj_name=''): 
    16931736    """
    16941737    try:
    16951738        spec = sage_getargspec(obj)
    1696         s = str(inspect.formatargspec(*spec))
     1739        s = str(formatargspec(*spec))
    16971740        s = s.strip('(').strip(')').strip()
    16981741        if s[:4] == 'self':
    16991742            s = s[4:]

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

Last edited 5 months ago by jhpalmieri (previous) (diff)

comment:3 Changed 5 months ago by vklein

  • Description modified (diff)

comment:4 Changed 5 months ago by vklein

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.

Last edited 5 months ago by vklein (previous) (diff)

comment:5 Changed 5 months ago by jhpalmieri

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.

Last edited 5 months ago by jhpalmieri (previous) (diff)

comment:6 Changed 5 months ago by jhpalmieri

  • Branch set to u/jhpalmieri/formatargspec

comment:7 Changed 5 months ago by jhpalmieri

  • Authors set to John Palmieri
  • Commit set to 56516cf95b775ffab3e6e85fd7c9c78ded927251
  • Status changed from new to needs_review

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:

56516cftrac 27971: implement our own version of formatargspec

comment:8 Changed 5 months ago by fbissey

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.

comment:9 Changed 5 months ago by jhpalmieri

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

comment:10 Changed 5 months ago by jhpalmieri

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

comment:11 Changed 5 months ago by jhpalmieri

  • Description modified (diff)

Ping?

comment:12 follow-up: Changed 5 months ago by vklein

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

comment:13 Changed 5 months ago by git

  • Commit changed from 56516cf95b775ffab3e6e85fd7c9c78ded927251 to bbd5b28fc8e451bea4f87c2f2b0c27586386262a

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

bbd5b28trac 27971: implement our own version of formatargspec

comment:14 in reply to: ↑ 12 Changed 5 months ago by jhpalmieri

Replying to vklein:

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

I modified this.

comment:15 Changed 5 months ago by vklein

  • Reviewers set to Vincent Klein
  • Status changed from needs_review to positive_review

Looks good to me.

comment:16 Changed 5 months ago by vbraun

  • Branch changed from u/jhpalmieri/formatargspec to bbd5b28fc8e451bea4f87c2f2b0c27586386262a
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:17 Changed 4 months ago by embray

  • Milestone changed from sage-8.8 to sage-8.9

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.

Note: See TracTickets for help on using tickets.