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

Give a warning when using citation.get_systems() without Cython profiling #23162

Closed
jdemeyer opened this issue Jun 7, 2017 · 8 comments
Closed

Comments

@jdemeyer
Copy link

jdemeyer commented Jun 7, 2017

sage.misc.citation.get_systems() is implemented by using cProfile to look at which modules implement the functions called when executing the code.

The problem is that this is totally unreliable when Cython is compiled without profiling support (which is the default). This doctest

sage: from sage.misc.citation import get_systems
sage: get_systems('((x+1)^2).expand()')
['ginac']

only works because Expression.expand() is called by Python instead of Cython. If that call would be inside some other Cython code, then Python's profiler would not detect it:

sage: cython('def callexpand(x): return x.expand()')
sage: from sage.misc.citation import get_systems
sage: get_systems('callexpand(((x+1)^2))')
[]

There is a problem because #22747 will "break" profiling even further as even the top-level call of Expression.expand() would not be detected as something to be entered in the profiler.

So here I propose simply to give a warning whenever get_systems() is used when profiling was not enabled.

CC: @seblabbe

Component: cython

Keywords: thursdaysbdx

Author: Jeroen Demeyer

Branch/Commit: ae120c5

Reviewer: Sébastien Labbé

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

@jdemeyer jdemeyer added this to the sage-8.0 milestone Jun 7, 2017
@jdemeyer
Copy link
Author

jdemeyer commented Jun 7, 2017

@jdemeyer
Copy link
Author

jdemeyer commented Jun 7, 2017

Commit: ae120c5

@jdemeyer
Copy link
Author

jdemeyer commented Jun 7, 2017

Author: Jeroen Demeyer

@jdemeyer
Copy link
Author

jdemeyer commented Jun 7, 2017

New commits:

ae120c5Give a warning when using citation.get_systems() without Cython profiling

@seblabbe
Copy link
Contributor

comment:4

Works for me on OSX 10.10. sage -t --long works. Code looks good. Documentation builds and looks fine.

@seblabbe
Copy link
Contributor

Reviewer: Sébastien Labbé

@seblabbe
Copy link
Contributor

Changed keywords from none to thursdaysbdx

@vbraun
Copy link
Member

vbraun commented Jun 15, 2017

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

3 participants