Skip to content

Commit

Permalink
Trac #32479: Remove monkey patching of inspect.isfunction in sage.__i…
Browse files Browse the repository at this point in the history
…nit__

"Monkey-patch `inspect.isfunction()` to support Cython functions."

Introduced in 2018 (8.3.beta2) in #25373 (for #24994).

However, this change has been rejected by upstream
(https://bugs.python.org/issue30071,
https://www.python.org/dev/peps/pep-0575/,
https://www.python.org/dev/peps/pep-0580/).

In this ticket:
- we remove the monkey-patching

- replace unnecessary uses of `inspect` in ordinary Sage library code by
other solutions

- replace remaining uses of `inspect.isfunction` by the new name of our
monkey-patched variant:
`sage.misc.sageinspect.is_function_or_cython_function`

(To be checked whether https://www.python.org/dev/peps/pep-0590,
superseding some of the above rejected PEPs, offers help; see also the
informational PEP https://www.python.org/dev/peps/pep-0579/)

Related:
- #26254 No signature shown in help for cythonized built-in methods
- #27578 Use `sphinx.util.inspect.Signature`, not
`sphinx.ext.autodoc.inspector.formatargspec`
- #31309 `sage_getargspec` mishandles keyword-only arguments

URL: https://trac.sagemath.org/32479
Reported by: mkoeppe
Ticket author(s): Matthias Koeppe
Reviewer(s): Michael Orlitzky
  • Loading branch information
Release Manager committed Sep 16, 2021
2 parents 9719668 + 2ba4c43 commit e307459
Show file tree
Hide file tree
Showing 13 changed files with 94 additions and 62 deletions.
41 changes: 14 additions & 27 deletions src/sage/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,44 +9,31 @@ def load_ipython_extension(*args):
sage.repl.ipython_extension.load_ipython_extension(*args)


# Monkey-patch inspect.isfunction() to support Cython functions.
# Deprecated leftover of monkey-patching inspect.isfunction() to support Cython functions.
# We cannot use lazy_import for the deprecation here.
def isfunction(obj):
"""
Check whether something is a function.
This is a variant of ``inspect.isfunction``:
We assume that anything which has a genuine ``__code__``
attribute (not using ``__getattr__`` overrides) is a function.
This is meant to support Cython functions.
This function is deprecated. Most uses of ``isfunction``
can be replaced by ``callable``.
EXAMPLES::
sage: from inspect import isfunction
sage: from sage import isfunction
sage: def f(): pass
sage: isfunction(f)
doctest:warning...
DeprecationWarning: sage.isfunction is deprecated; use callable or sage.misc.sageinspect.is_function_or_cython_function instead
See https://trac.sagemath.org/32479 for details.
True
sage: isfunction(lambda x:x)
True
sage: from sage.categories.coercion_methods import _mul_parent
sage: isfunction(_mul_parent)
True
sage: isfunction(Integer.digits) # unbound method
False
sage: isfunction(Integer(1).digits) # bound method
False
Verify that ipywidgets can correctly determine signatures of Cython
functions::
sage: from ipywidgets.widgets.interaction import signature
sage: from sage.dynamics.complex_dynamics.mandel_julia_helper import fast_mandelbrot_plot
sage: signature(fast_mandelbrot_plot) # random
<IPython.utils._signatures.Signature object at 0x7f3ec8274e10>
"""
# We use type(obj) instead of just obj to avoid __getattr__().
# Some types, like methods, will return the __code__ of the
# underlying function in __getattr__() but we don't want to
# detect those as functions.
return hasattr(type(obj), "__code__")

import inspect
inspect.isfunction = isfunction
from sage.misc.superseded import deprecation
deprecation(32479, "sage.isfunction is deprecated; use callable or sage.misc.sageinspect.is_function_or_cython_function instead")
from sage.misc.sageinspect import is_function_or_cython_function
return is_function_or_cython_function(obj)
7 changes: 3 additions & 4 deletions src/sage/algebras/steenrod/steenrod_algebra_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,6 @@ def normalize_profile(profile, precision=None, truncation_type='auto', p=2, gene
...
ValueError: Invalid profile
"""
from inspect import isfunction
from sage.rings.infinity import Infinity
if truncation_type == 'zero':
truncation_type = 0
Expand All @@ -482,7 +481,7 @@ def normalize_profile(profile, precision=None, truncation_type='auto', p=2, gene
while profile and profile[-1] == truncation_type:
profile = profile[:-1]
new_profile = tuple(profile)
elif isfunction(profile):
elif callable(profile):
# profile is a function: turn it into a tuple. if
# truncation_type not specified, set it to 'infinity' if
# the function is ever infinite; otherwise set it to
Expand Down Expand Up @@ -522,7 +521,7 @@ def normalize_profile(profile, precision=None, truncation_type='auto', p=2, gene
while e and e[-1] == truncation_type:
e = e[:-1]
e = tuple(e)
elif isfunction(e):
elif callable(e):
# e is a function: turn it into a tuple. if
# truncation_type not specified, set it to 'infinity'
# if the function is ever infinite; otherwise set it
Expand All @@ -541,7 +540,7 @@ def normalize_profile(profile, precision=None, truncation_type='auto', p=2, gene
if isinstance(k, (list, tuple)):
# k is a list or tuple: use it as is.
k = tuple(k)
elif isfunction(k):
elif callable(k):
# k is a function: turn it into a tuple.
if precision is None:
k_precision = 100
Expand Down
18 changes: 10 additions & 8 deletions src/sage/calculus/integration.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ AUTHORS:

from cysignals.signals cimport sig_on, sig_off
from memory_allocator cimport MemoryAllocator
import inspect

from sage.rings.real_double import RDF
from sage.libs.gsl.all cimport *
Expand Down Expand Up @@ -582,19 +581,21 @@ def monte_carlo_integral(func, xl, xu, size_t calls, algorithm='plain',
_xu[i] = <double> xu[i]

if not callable(func):
# constant
# constant. Note that all Expression objects are callable.
v = float(1)
for i in range(dim):
v *= _xu[i] - _xl[i]
return (v * <double?> func, 0.0)

elif not isinstance(func, Wrapper_rdf):
if inspect.isfunction(func):
vars = sage_getargspec(func)[0]
elif hasattr(func, 'arguments'):
# func is either an Expression or another callable.
try:
vars = func.arguments()
else:
vars = func.variables()
except AttributeError:
try:
vars = func.variables()
except AttributeError:
vars = sage_getargspec(func)[0]

target_dim = dim + len(params)
if len(vars) < target_dim:
Expand All @@ -611,7 +612,8 @@ def monte_carlo_integral(func, xl, xu, size_t calls, algorithm='plain',
"more items in upper and lower limits"
).format(len(vars), tuple(vars), target_dim))

if not inspect.isfunction(func):
from sage.symbolic.expression import is_Expression
if is_Expression(func):
if params:
to_sub = dict(zip(vars[-len(params):], params))
func = func.subs(to_sub)
Expand Down
5 changes: 3 additions & 2 deletions src/sage/misc/rest_index_of_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import inspect

from sage.misc.sageinspect import _extract_embedded_position
from sage.misc.sageinspect import is_function_or_cython_function as _isfunction


def gen_rest_table_index(obj, names=None, sort=True, only_local_functions=True):
Expand Down Expand Up @@ -172,10 +173,10 @@ def gen_rest_table_index(obj, names=None, sort=True, only_local_functions=True):
link = ":meth:`~{module}.{cls}.{func}`".format(
module=e.im_class.__module__, cls=e.im_class.__name__,
func=fname(e))
elif inspect.isfunction(e) and inspect.isclass(obj):
elif _isfunction(e) and inspect.isclass(obj):
link = ":meth:`~{module}.{cls}.{func}`".format(
module=obj.__module__, cls=obj.__name__, func=fname(e))
elif inspect.isfunction(e):
elif _isfunction(e):
link = ":func:`~{module}.{func}`".format(
module=e.__module__, func=fname(e))
else:
Expand Down
49 changes: 48 additions & 1 deletion src/sage/misc/sageinspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,53 @@ def foo(unsigned int x=1, a=')"', b={not (2+1==3):'bar'}, *args, **kwds): return
pass


def is_function_or_cython_function(obj):
"""
Check whether something is a function.
This is a variant of :func:`inspect.isfunction`:
We assume that anything which has a genuine ``__code__``
attribute (not using ``__getattr__`` overrides) is a function.
This is meant to support Cython functions.
Think twice before using this function (or any function from the
:mod:`inspect` or :mod:`sage.misc.sageinspect` modules). Most uses of
:func:`inspect.isfunction` in ordinary library code can be replaced by
:func:`callable`.
EXAMPLES::
sage: from sage.misc.sageinspect import is_function_or_cython_function
sage: def f(): pass
sage: is_function_or_cython_function(f)
True
sage: is_function_or_cython_function(lambda x:x)
True
sage: from sage.categories.coercion_methods import _mul_parent
sage: is_function_or_cython_function(_mul_parent)
True
sage: is_function_or_cython_function(Integer.digits) # unbound method
False
sage: is_function_or_cython_function(Integer(1).digits) # bound method
False
TESTS:
Verify that ipywidgets can correctly determine signatures of Cython
functions::
sage: from ipywidgets.widgets.interaction import signature
sage: from sage.dynamics.complex_dynamics.mandel_julia_helper import fast_mandelbrot_plot
sage: signature(fast_mandelbrot_plot) # random
<IPython.utils._signatures.Signature object at 0x7f3ec8274e10>
"""
# We use type(obj) instead of just obj to avoid __getattr__().
# Some types, like methods, will return the __code__ of the
# underlying function in __getattr__() but we don't want to
# detect those as functions.
return hasattr(type(obj), "__code__")


def loadable_module_extension():
r"""
Return the filename extension of loadable modules, including the dot.
Expand Down Expand Up @@ -2175,7 +2222,7 @@ class ParentMethods:

if inspect.ismethod(obj):
obj = obj.__func__
if inspect.isfunction(obj):
if is_function_or_cython_function(obj):
obj = obj.__code__
if inspect.istraceback(obj):
obj = obj.tb_frame
Expand Down
3 changes: 1 addition & 2 deletions src/sage/modules/free_module_homspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@
from sage.structure.element import is_Matrix
from sage.matrix.constructor import matrix, identity_matrix
from sage.matrix.matrix_space import MatrixSpace
from inspect import isfunction
from sage.misc.cachefunc import cached_method


Expand Down Expand Up @@ -202,7 +201,7 @@ def __call__(self, A, **kwds):
# generators of the domain to the elements of A.
C = self.codomain()
try:
if isfunction(A):
if callable(A):
v = [C(A(g)) for g in self.domain().gens()]
A = matrix([C.coordinates(a) for a in v], ncols=C.rank())
if side == "right":
Expand Down
3 changes: 1 addition & 2 deletions src/sage/modules/vector_space_homspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@
# http://www.gnu.org/licenses/
####################################################################################

import inspect
import sage.matrix.all as matrix
import sage.modules.free_module_homspace

Expand Down Expand Up @@ -375,7 +374,7 @@ def __call__(self, A, check=True, **kwds):
pass
elif is_VectorSpaceMorphism(A):
A = A.matrix()
elif inspect.isfunction(A):
elif callable(A):
try:
images = [A(g) for g in D.basis()]
except (ValueError, TypeError, IndexError) as e:
Expand Down
5 changes: 2 additions & 3 deletions src/sage/modules/vector_space_morphism.py
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,6 @@ def linear_transformation(arg0, arg1=None, arg2=None, side='left'):
from sage.categories.homset import Hom
from sage.symbolic.ring import SR
from sage.modules.vector_callable_symbolic_dense import Vector_callable_symbolic_dense
from inspect import isfunction

if not side in ['left', 'right']:
raise ValueError("side must be 'left' or 'right', not {0}".format(side))
Expand Down Expand Up @@ -734,8 +733,6 @@ def linear_transformation(arg0, arg1=None, arg2=None, side='left'):
arg2 = arg2.transpose()
elif isinstance(arg2, (list, tuple)):
pass
elif isfunction(arg2):
pass
elif isinstance(arg2, Vector_callable_symbolic_dense):
args = arg2.parent().base_ring()._arguments
exprs = arg2.change_ring(SR)
Expand All @@ -758,6 +755,8 @@ def linear_transformation(arg0, arg1=None, arg2=None, side='left'):
except (ArithmeticError, TypeError) as e:
msg = 'some image of the function is not in the codomain, because\n' + e.args[0]
raise ArithmeticError(msg)
elif callable(arg2):
pass
else:
msg = 'third argument must be a matrix, function, or list of images, not {0}'
raise TypeError(msg.format(arg2))
Expand Down
5 changes: 2 additions & 3 deletions src/sage/plot/plot3d/plot3d.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ def f(x,y): return math.exp(x/5)*math.cos(y)
#
# https://www.gnu.org/licenses/
# ****************************************************************************
import inspect

from .tri_plot import TrianglePlot
from .index_face_set import IndexFaceSet
Expand All @@ -153,7 +152,7 @@ def f(x,y): return math.exp(x/5)*math.cos(y)
from sage.ext.fast_eval import fast_float_arg

from sage.functions.trig import cos, sin
from sage.misc.sageinspect import sage_getargspec
from sage.misc.sageinspect import sage_getargspec, is_function_or_cython_function


class _Coordinates(object):
Expand Down Expand Up @@ -394,7 +393,7 @@ def _find_arguments_for_callable(func):
sage: _find_arguments_for_callable(operator.add)
[]
"""
if inspect.isfunction(func):
if is_function_or_cython_function(func):
pass
elif hasattr(func, 'arguments'):
# Might be a symbolic function with arguments
Expand Down
4 changes: 2 additions & 2 deletions src/sage/plot/plot_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,8 @@ def plot_slope_field(f, xrange, yrange, **kwds):
slope_options.update(kwds)

from sage.functions.all import sqrt
from inspect import isfunction
if isfunction(f):
from sage.misc.sageinspect import is_function_or_cython_function
if is_function_or_cython_function(f):
norm_inverse = lambda x,y: 1/sqrt(f(x, y)**2+1)
f_normalized = lambda x,y: f(x, y)*norm_inverse(x, y)
else:
Expand Down
4 changes: 2 additions & 2 deletions src/sage/plot/streamline_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,8 @@ def streamline_plot(f_g, xrange, yrange, **options):
(f,g) = f_g
else:
from sage.functions.all import sqrt
from inspect import isfunction
if isfunction(f_g):
from sage.misc.sageinspect import is_function_or_cython_function
if is_function_or_cython_function(f_g):
f = lambda x,y: 1 / sqrt(f_g(x, y)**2 + 1)
g = lambda x,y: f_g(x, y) * f(x, y)
else:
Expand Down
3 changes: 1 addition & 2 deletions src/sage/symbolic/expression.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,6 @@ More sanity tests::
from cysignals.signals cimport sig_on, sig_off
from sage.ext.cplusplus cimport ccrepr, ccreadstr

from inspect import isfunction
import operator
import sage.rings.integer
import sage.rings.rational
Expand Down Expand Up @@ -13378,7 +13377,7 @@ cdef get_dynamic_class_for_function(unsigned serial):
# Fix methods from eval_methods, wrapping them to extract
# the operands and pass them as arguments
for name, meth in eval_methods.__dict__.items():
if not isfunction(meth):
if not callable(meth):
continue
meth = _eval_on_operands(meth)
setattr(cls, name, meth)
Expand Down
9 changes: 5 additions & 4 deletions src/sage_docbuild/ext/sage_autodoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@

from sage.misc.sageinspect import (sage_getdoc_original,
sage_getargspec, isclassinstance,
sage_formatargspec)
sage_formatargspec,
is_function_or_cython_function)
from sage.misc.lazy_import import LazyImport

# This is used to filter objects of classes that inherit from
Expand Down Expand Up @@ -1076,7 +1077,7 @@ def can_document_member(cls, member, membername, isattr, parent):
# whose doc string coincides with that of f and is thus different from
# that of the class CachedFunction. In that situation, we want that f is documented.
# This is part of trac #9976.
return (inspect.isfunction(member) or inspect.isbuiltin(member)
return (is_function_or_cython_function(member) or inspect.isbuiltin(member)
or (isclassinstance(member)
and sage_getdoc_original(member) != sage_getdoc_original(member.__class__)))

Expand Down Expand Up @@ -1223,7 +1224,7 @@ def format_args(self):
# __init__ written in C?
if initmeth is None or \
is_builtin_class_method(self.object, '__init__') or \
not(inspect.ismethod(initmeth) or inspect.isfunction(initmeth)):
not(inspect.ismethod(initmeth) or is_function_or_cython_function(initmeth)):
return None
try:
argspec = sage_getargspec(initmeth)
Expand Down Expand Up @@ -1486,7 +1487,7 @@ class AttributeDocumenter(DocstringStripSignatureMixin, ClassLevelDocumenter):

@staticmethod
def is_function_or_method(obj):
return inspect.isfunction(obj) or inspect.isbuiltin(obj) or inspect.ismethod(obj)
return is_function_or_cython_function(obj) or inspect.isbuiltin(obj) or inspect.ismethod(obj)

@classmethod
def can_document_member(cls, member, membername, isattr, parent):
Expand Down

0 comments on commit e307459

Please sign in to comment.