sage_getargspec mishandles keyword-only arguments
Opened this issue · 10 comments
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
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))
+```
+
+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
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.
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.
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](https://docs.python.org/3/library/inspect.html#inspect.signature)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](https://docs.python.org/3/library/inspect.html#inspect.signature)
+In this ticket, we deprecate this function and replace all uses by [inspect.signature](https://docs.python.org/3/library/inspect.html#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
+```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:
f6a0eb9 | src/sage/misc/sageinspect.py (sage_getargspec): In doctests, show result of inspect.signature too |
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](https://docs.python.org/3/library/inspect.html#inspect.signature):
+In this ticket, we deprecate this function and replace all uses by a new function `signature` that is compatible with [inspect.signature](https://docs.python.org/3/library/inspect.html#inspect.signature):
```
$ git grep -l sage_getargspec