sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 481 forks source link

sage_getargspec mishandles keyword-only arguments #31309

Open mkoeppe opened 3 years ago

mkoeppe commented 3 years ago

sage_getargspec seems broken for functions with keyword-only arguments.

In #31307, the sphinx documentation for

@staticmethod
def random_element(m, n, bound=5, special_probability=0.2,
                   *, is_primal=True, **kwds):

is generated as

static random_element(m, n, bound, special_probability=5, is_primal=0.2, **kwds)
Construct a random InteractiveLPProblemStandardForm.

What's happening here is:

sage: def random_element(m, n, bound=5, special_probability=0.2, 
....:                    *, is_primal=True, **kwds): 
....:     return 1 
sage: from sage.misc.sageinspect import sage_getargspec                                                                                                                                                                    
sage: sage_getargspec(random_element)                                                                                                                                                                                      
ArgSpec(args=['m', 'n', 'bound', 'special_probability', 'is_primal'], varargs=None, keywords='kwds', defaults=(5, 0.200000000000000))

In this ticket, we deprecate this function and replace all uses by a new function signature that is compatible with inspect.signature:

$ git grep -l sage_getargspec
src/sage/calculus/integration.pyx
src/sage/calculus/ode.pyx
src/sage/coding/abstract_code.py
src/sage/combinat/finite_state_machine.py
src/sage/crypto/mq/rijndael_gf.py
src/sage/docs/conf.py
src/sage/interfaces/singular.py
src/sage/libs/singular/standard_options.py
src/sage/misc/cachefunc.pyx
src/sage/misc/decorators.py
src/sage/misc/function_mangling.pyx
src/sage/misc/lazy_import.pyx
src/sage/misc/sageinspect.py
src/sage/parallel/decorate.py
src/sage/plot/plot3d/plot3d.py
src/sage/repl/ipython_extension.py
src/sage/sets/set_from_iterator.py
src/sage/tests/finite_poset.py
src/sage_docbuild/ext/sage_autodoc.py

CC: @yuan-zhou @tobiasdiez @kwankyu @fchapoton @tscrim

Component: documentation

Branch/Commit: u/mkoeppe/sage_getargspec_mishandles_keyword_only_arguments @ f6a0eb9

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

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -14,4 +14,15 @@
 Construct a random InteractiveLPProblemStandardForm.

+What's happening here is:

+ +sage: def random_element(m, n, bound=5, special_probability=0.2, +....: *, is_primal=True, **kwds): +....: return 1 +sage: from sage.misc.sageinspect import sage_getargspec +sage: sage_getargspec(random_element) +ArgSpec(args=['m', 'n', 'bound', 'special_probability', 'is_primal'], varargs=None, keywords='kwds', defaults=(5, 0.200000000000000)) + + +

mkoeppe commented 3 years ago
comment:2

The implementation of sage_getargspec uses inspect.getargs -- which so old that it is not even deprecated - https://docs.python.org/3/library/inspect.html

mkoeppe commented 3 years ago
comment:3

If we can't get rid of sage_getargspec (#30884?), we should make a version of sage_getargspec that returns a FullArgSpec (see https://docs.python.org/3/library/inspect.html#inspect.getfullargspec) or Signature (https://docs.python.org/3/library/inspect.html#inspect.signature) instead of an ArgSpec.

mkoeppe commented 3 years ago
comment:4

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -25,4 +25,4 @@
 ArgSpec(args=['m', 'n', 'bound', 'special_probability', 'is_primal'], varargs=None, keywords='kwds', defaults=(5, 0.200000000000000))

- +In this ticket, we deprecate this function and replace all uses by inspect.signature

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -25,4 +25,27 @@
 ArgSpec(args=['m', 'n', 'bound', 'special_probability', 'is_primal'], varargs=None, keywords='kwds', defaults=(5, 0.200000000000000))

-In this ticket, we deprecate this function and replace all uses by inspect.signature +In this ticket, we deprecate this function and replace all uses by inspect.signature: + + +$ git grep -l sage_getargspec +src/sage/calculus/integration.pyx +src/sage/calculus/ode.pyx +src/sage/coding/abstract_code.py +src/sage/combinat/finite_state_machine.py +src/sage/crypto/mq/rijndael_gf.py +src/sage/docs/conf.py +src/sage/interfaces/singular.py +src/sage/libs/singular/standard_options.py +src/sage/misc/cachefunc.pyx +src/sage/misc/decorators.py +src/sage/misc/function_mangling.pyx +src/sage/misc/lazy_import.pyx +src/sage/misc/sageinspect.py +src/sage/parallel/decorate.py +src/sage/plot/plot3d/plot3d.py +src/sage/repl/ipython_extension.py +src/sage/sets/set_from_iterator.py +src/sage/tests/finite_poset.py +src/sage_docbuild/ext/sage_autodoc.py +

mkoeppe commented 3 years ago

Branch: u/mkoeppe/sage_getargspec_mishandles_keyword_only_arguments

mkoeppe commented 3 years ago

Commit: f6a0eb9

mkoeppe commented 3 years ago
comment:9

Without #26254, inspect.signature is not able to return signatures of Cython-defined functions -- so the plan to just replace all uses of sage_getargspec by inspect.signature cannot work.

Instead we should create a function signature in sage.misc.sageinspect that is compatible with inspect.signature and switch all uses of sage_getargspec to that.


New commits:

f6a0eb9src/sage/misc/sageinspect.py (sage_getargspec): In doctests, show result of inspect.signature too
mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -25,7 +25,7 @@
 ArgSpec(args=['m', 'n', 'bound', 'special_probability', 'is_primal'], varargs=None, keywords='kwds', defaults=(5, 0.200000000000000))

-In this ticket, we deprecate this function and replace all uses by inspect.signature: +In this ticket, we deprecate this function and replace all uses by a new function signature that is compatible with inspect.signature:

 $ git grep -l sage_getargspec