#14017: Determine the correct argspec for python functions defined in cython
files
------------------------------------------------+---------------------------
Reporter: SimonKing | Owner: jason
Type: defect | Status: needs_review
Priority: major | Milestone: sage-5.7
Component: misc | Resolution:
Keywords: introspection cython argspec | Work issues:
Report Upstream: N/A | Reviewers: Travis
Scrimshaw
Authors: Simon King | Merged in:
Dependencies: | Stopgaps:
------------------------------------------------+---------------------------
Comment (by SimonKing):
Hi Travis,
Replying to [comment:10 tscrim]:
> So it's not a requirement to actually check bad syntax, but only needs
to guarantee that it will parse correct syntax. Correct?
Correct. The patch fixes cases in which this used not to be the case. But
I think it is a bug if correct syntax is not parsed.
> Sorry I was unclear; I was thinking of if the function was in python:
> {{{
> sage: def test(): return 1
> ....:
> sage: def f(x,y={test():2},**kwds): pass
> ....:
> sage: sage_getargspec(f)
> ArgSpec(args=['x', 'y'], varargs=None, keywords='kwds', defaults=({1:
2},))
> }}}
That's indeed tricky.
This example does work for Python functions, because inspect.getargspec
can read off the default values from some attribute of the function --
hence, reading source code is not involved for Python.
But if you put the same definition in a .pyx file, then inspect.getargspec
does not work, unless one compiles everything with the "binding" option
(which is probably bad for performance). Hence, source code parsing
probably is the only way to find the arguments. But as soon as you use
external functions to define default values, source code parsing must fail
sooner or later.
In this example, `_sage_getargspec_from_ast` (and thus
`_sage_getargspec_cython`) would return `{None: 2}` and not `{1: 2}` as
default value.
I see no safe way to overcome this limitation. But note that knowing the
correct default values is less critical than knowing the argument names.
The bug that made me create this ticket was: Currently the argument names
are not correctly determined, in some situations.
> I think I'm now starting I understand what the issue is. However I don't
see how evaluating the default argument string in the modules namespace
could cause an issue
No, in examples of that kind, source code parsing ''must'' fail at some
point. Two examples:
* If test() in the example above is a cdef function that is not declared
in the .pxd file, then `_sage_getargspec_from_ast` (which returns the
incorrect default) can not import it. Hence, it would be impossible to
simply call `test()` and see what it returns.
* It is easy to write a function whose return value changes over time. If
`test()` is such function, then the value used to define the default
argument of f would differ from the value deduced by
`_sage_getargspec_from_ast`.
> > > As for the patch, two quick things. Is there any reason to have that
large commented out code block?
> >
> > Oops, sorry, I'll remove that block and will submit a new patch after
dinner.
>
> Thanks.
This will be my next post - dinner was quite long...
> > > Also I believe we should try to be python 3 compliant as much as
possible, so the exceptions should be `raise ExceptionType("msg")`.
> >
> > Never heard of that rule. Could you ask on sage-devel whether this
rule should be encouraged?
>
> Here's the topic: https://groups.google.com/forum/?fromgroups=#!topic
/sage-devel/P35X-WjDuK4
Thanks!
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/14017#comment:11>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
