Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sage_getargspec mishandles keyword-only arguments #31309

Open
mkoeppe opened this issue Jan 31, 2021 · 10 comments
Open

sage_getargspec mishandles keyword-only arguments #31309

mkoeppe opened this issue Jan 31, 2021 · 10 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Jan 31, 2021

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 mkoeppe added this to the sage-9.3 milestone Jan 31, 2021
@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Jan 31, 2021

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
Copy link
Member Author

mkoeppe commented Jan 31, 2021

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
Copy link
Member Author

mkoeppe commented Mar 24, 2021

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 mkoeppe modified the milestones: sage-9.3, sage-9.4 Mar 24, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Sep 6, 2021

@mkoeppe
Copy link
Member Author

mkoeppe commented Sep 6, 2021

Commit: f6a0eb9

@mkoeppe
Copy link
Member Author

mkoeppe commented Sep 6, 2021

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

This comment has been minimized.

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 14, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 7, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Aug 31, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.8, sage-9.9 Jan 7, 2023
kryzar pushed a commit to kryzar/sage that referenced this issue Feb 6, 2023
We rebase `sage_autodoc` to Sphinx 5.3.0.

This is a step toward eventual removal of sage_autodoc in sagemath#30893, a
customization of Sphinx autodoc extension for Sage objects.

Other related tickets are sagemath#27578. sagemath#30884, sagemath#31309, in this regard.

URL: https://trac.sagemath.org/34730
Reported by: klee
Ticket author(s): Kwankyu Lee
Reviewer(s): Tobias Diez
@mkoeppe mkoeppe removed this from the sage-10.0 milestone Apr 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant