Skip to content

Commit

Permalink
Trac #33017: LazyImport.__instancecheck__, __subclasscheck__: Return …
Browse files Browse the repository at this point in the history
…False on ImportError

No object is an instance of a class that cannot be imported.

No class is a subclass of a class that cannot be imported.

Hence, we change `LazyImport` so that the special methods
`__instancecheck__`, `__subclasscheck__` catch `ImportError` and just
return False.

(We do not use the stronger versions of these two statements: "No object
is an instance of a class that has not been imported yet (i.e., the
module name is not in `sys.modules`)." These are true only if there are
no re-exports.)

With this change to `LazyImport`, we have another instrument for
modularization of code that uses `isinstance` checks. (The use of
`isinstance` with ABCs (#32566) is preferable because it avoids
importing an otherwise unused implementation class.)

URL: https://trac.sagemath.org/33017
Reported by: mkoeppe
Ticket author(s): Matthias Koeppe
Reviewer(s): Jonathan Kliem
  • Loading branch information
Release Manager committed Apr 9, 2022
2 parents ae06fa4 + 7adef96 commit 204cbf0
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 7 deletions.
77 changes: 72 additions & 5 deletions src/doc/en/developer/packaging_sage_library.rst
Original file line number Diff line number Diff line change
Expand Up @@ -266,19 +266,86 @@ distribution -- which then must be declared as a run-time dependency.
provides abstract base classes for many ring (parent) classes,
including :class:`sage.rings.abc.pAdicField`. So we can replace::

from sage.rings.padics.generic_nodes import pAdicField # heavy dependencies
isinstance(object, pAdicField)
from sage.rings.padics.generic_nodes import pAdicFieldGeneric # heavy dependencies
isinstance(object, pAdicFieldGeneric)

and::

from sage.rings.padics.generic_nodes import pAdicField # heavy dependencies
is_pAdicField(object) # deprecated
from sage.rings.padics.generic_nodes import is_pAdicField # heavy dependencies
is_pAdicField(object) # deprecated

by::

import sage.rings.abc # no dependencies
import sage.rings.abc # no dependencies
isinstance(object, sage.rings.abc.pAdicField)

Note that going through the abstract base class only incurs a small
performance penalty::

sage: object = Qp(5)

sage: from sage.rings.padics.generic_nodes import pAdicFieldGeneric
sage: %timeit isinstance(object, pAdicFieldGeneric) # fast # not tested
68.7 ns ± 2.29 ns per loop (...)

sage: import sage.rings.abc
sage: %timeit isinstance(object, sage.rings.abc.pAdicField) # also fast # not tested
122 ns ± 1.9 ns per loop (...)

- If it is not possible or desired to create an abstract base class for
``isinstance`` testing (for example, when the class is defined in some
external package), other solutions need to be used.

Note that Python caches successful module imports, but repeating an
unsuccessful module import incurs a cost every time::

sage: from sage.schemes.generic.scheme import Scheme
sage: sZZ = Scheme(ZZ)

sage: def is_Scheme_or_Pluffe(x):
....: if isinstance(x, Scheme):
....: return True
....: try:
....: from xxxx_does_not_exist import Pluffe # slow on every call
....: except ImportError:
....: return False
....: return isinstance(x, Pluffe)

sage: %timeit is_Scheme_or_Pluffe(sZZ) # fast # not tested
111 ns ± 1.15 ns per loop (...)

sage: %timeit is_Scheme_or_Pluffe(ZZ) # slow # not tested
143 µs ± 2.58 µs per loop (...)

The :func:`~sage.misc.lazy_import.lazy_import` mechanism can be used to simplify
this pattern via the :meth:`~sage.misc.lazy_import.LazyImport.__instancecheck__`
method and has similar performance characteristics::

sage: lazy_import('xxxx_does_not_exist', 'Pluffe')

sage: %timeit isinstance(sZZ, (Scheme, Pluffe)) # fast # not tested
95.2 ns ± 0.636 ns per loop (...)

sage: %timeit isinstance(ZZ, (Scheme, Pluffe)) # slow # not tested
158 µs ± 654 ns per loop (...)

It is faster to do the import only once, for example when loading the module,
and to cache the failure. We can use the following idiom, which makes
use of the fact that ``isinstance`` accepts arbitrarily nested lists
and tuples of types::

sage: try:
....: from xxxx_does_not_exist import Pluffe # runs once
....: except ImportError:
....: # Set to empty tuple of types for isinstance
....: Pluffe = ()

sage: %timeit isinstance(sZZ, (Scheme, Pluffe)) # fast # not tested
95.9 ns ± 1.52 ns per loop (...)

sage: %timeit isinstance(ZZ, (Scheme, Pluffe)) # fast # not tested
126 ns ± 1.9 ns per loop (...)


Other runtime dependencies
--------------------------
Expand Down
22 changes: 20 additions & 2 deletions src/sage/misc/lazy_import.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -912,8 +912,17 @@ cdef class LazyImport(object):
sage: lazy_import('sage.rings.rational_field', 'RationalField')
sage: isinstance(QQ, RationalField)
True
No object is an instance of a class that cannot be imported::
sage: lazy_import('sage.xxxxx_does_not_exist', 'DoesNotExist')
sage: isinstance(QQ, DoesNotExist)
False
"""
return isinstance(x, self.get_object())
try:
return isinstance(x, self.get_object())
except ImportError:
return False

def __subclasscheck__(self, x):
"""
Expand All @@ -924,8 +933,17 @@ cdef class LazyImport(object):
sage: lazy_import('sage.structure.parent', 'Parent')
sage: issubclass(RationalField, Parent)
True
No class is a subclass of a class that cannot be imported::
sage: lazy_import('sage.xxxxx_does_not_exist', 'DoesNotExist')
sage: issubclass(RationalField, DoesNotExist)
False
"""
return issubclass(x, self.get_object())
try:
return issubclass(x, self.get_object())
except ImportError:
return False


def lazy_import(module, names, as_=None, *,
Expand Down

0 comments on commit 204cbf0

Please sign in to comment.