LazyImport.__instancecheck__, __subclasscheck__: Return False on ImportError
Closed this issue · 20 comments
mkoeppe commented
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.)
Depends on #32899
Component: refactoring
Author: Matthias Koeppe
Branch: 7adef96
Reviewer: Jonathan Kliem
Issue created by migration from https://trac.sagemath.org/ticket/33017
mkoeppe commented
mkoeppe commented
Author: Matthias Koeppe
mkoeppe commented
New commits:
447df69 | LazyImport.__instancecheck__, __subclasscheck__: Return False on ImportError |
mkoeppe commented
Description changed:
---
+++
@@ -2,3 +2,9 @@
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 is 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.)
+mkoeppe commented
Description changed:
---
+++
@@ -4,7 +4,7 @@
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 is in `sys.modules`." These are true only if there are no re-exports.)
+(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.)
kliem commented
comment:5
It's not fast I suspect, as it tries an import and fails every time. But it is correct at least.
kliem commented
Reviewer: Jonathan Kliem
mkoeppe commented
comment:8
I'll add some documentation to discuss performance
sagetrac-git commented
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
821d16e | src/doc/en/developer/packaging_sage_library.rst: Add bootstrap to testing instructions to avoid the trap of #32868 |
77c957f | src/doc/en/developer/packaging_sage_library.rst: Put hierarchy section one level higher |
9375ef8 | pkgs/sagemath-standard/tox.ini: Use SAGE_VENV or venv symlink to find wheels |
c6fc111 | Prettier diagram |
7d6a44c | Use :mod: as markup for packages/modules |
f83d424 | Improve ABC example |
817a8d0 | src/doc/en/developer/packaging_sage_library.rst: More :mod: and :class: markup |
b555735 | src/doc/en/developer/packaging_sage_library.rst: Link to pypi.org and to documentation of packaging metadata |
fb151ea | Merge #32899 |
907f8e8 | src/doc/en/developer/packaging_sage_library.rst: Expand on LazyImport, add timings |
sagetrac-git commented
kliem commented
comment:10
No patchbot yet, due to
https://groups.google.com/g/sage-devel/c/epajAbPvg6s/m/-yWW1gFyCAAJ
kliem commented
comment:11
Edited to remove unrelated failure.
$ sage -t --long --random-seed=57900501385357039198738698054253811820 src/doc/en/developer/packaging_sage_library.rst # 8 doctests failed
too many failed tests, not using stored timings
Running doctests with ID 2021-12-17-15-38-43-8b38ad21.
Git branch: test_33017
Using --optional=4ti2,build,debian,dochtml,e_antic,latte_int,lidia,normaliz,pip,pynormaliz,sage,sage.geometry.polyhedron,sage.rings.real_double,sage_spkg
Doctesting 1 file.
sage -t --long --random-seed=57900501385357039198738698054253811820 src/doc/en/developer/packaging_sage_library.rst
**********************************************************************
File "src/doc/en/developer/packaging_sage_library.rst", line 285, in doc.en.developer.packaging_sage_library
Failed example:
%timeit isinstance(object, pAdicFieldGeneric) # fast # random
Exception raised:
Traceback (most recent call last):
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
compiled = compiler(example)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 662, in compiler
compileflags, 1)
File "<doctest doc.en.developer.packaging_sage_library[2]>", line 1
%timeit isinstance(object, pAdicFieldGeneric) # fast # random
^
SyntaxError: invalid syntax
**********************************************************************
File "src/doc/en/developer/packaging_sage_library.rst", line 289, in doc.en.developer.packaging_sage_library
Failed example:
%timeit isinstance(object, sage.rings.abc.pAdicField) # also fast # random
Exception raised:
Traceback (most recent call last):
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
compiled = compiler(example)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 662, in compiler
compileflags, 1)
File "<doctest doc.en.developer.packaging_sage_library[4]>", line 1
%timeit isinstance(object, sage.rings.abc.pAdicField) # also fast # random
^
SyntaxError: invalid syntax
**********************************************************************
File "src/doc/en/developer/packaging_sage_library.rst", line 311, in doc.en.developer.packaging_sage_library
Failed example:
%timeit is_Scheme_or_Pluffe(sZZ) # fast # random
Exception raised:
Traceback (most recent call last):
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
compiled = compiler(example)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 662, in compiler
compileflags, 1)
File "<doctest doc.en.developer.packaging_sage_library[3]>", line 1
%timeit is_Scheme_or_Pluffe(sZZ) # fast # random
^
SyntaxError: invalid syntax
**********************************************************************
File "src/doc/en/developer/packaging_sage_library.rst", line 314, in doc.en.developer.packaging_sage_library
Failed example:
%timeit is_Scheme_or_Pluffe(ZZ) # slow # random
Exception raised:
Traceback (most recent call last):
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
compiled = compiler(example)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 662, in compiler
compileflags, 1)
File "<doctest doc.en.developer.packaging_sage_library[4]>", line 1
%timeit is_Scheme_or_Pluffe(ZZ) # slow # random
^
SyntaxError: invalid syntax
**********************************************************************
File "src/doc/en/developer/packaging_sage_library.rst", line 323, in doc.en.developer.packaging_sage_library
Failed example:
%timeit isinstance(sZZ, (Scheme, Pluffe)) # fast # random
Exception raised:
Traceback (most recent call last):
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
compiled = compiler(example)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 662, in compiler
compileflags, 1)
File "<doctest doc.en.developer.packaging_sage_library[1]>", line 1
%timeit isinstance(sZZ, (Scheme, Pluffe)) # fast # random
^
SyntaxError: invalid syntax
**********************************************************************
File "src/doc/en/developer/packaging_sage_library.rst", line 326, in doc.en.developer.packaging_sage_library
Failed example:
%timeit isinstance(ZZ, (Scheme, Pluffe)) # slow # random
Exception raised:
Traceback (most recent call last):
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
compiled = compiler(example)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 662, in compiler
compileflags, 1)
File "<doctest doc.en.developer.packaging_sage_library[2]>", line 1
%timeit isinstance(ZZ, (Scheme, Pluffe)) # slow # random
^
SyntaxError: invalid syntax
**********************************************************************
File "src/doc/en/developer/packaging_sage_library.rst", line 340, in doc.en.developer.packaging_sage_library
Failed example:
%timeit isinstance(sZZ, (Scheme, Pluffe)) # fast # random
Exception raised:
Traceback (most recent call last):
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
compiled = compiler(example)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 662, in compiler
compileflags, 1)
File "<doctest doc.en.developer.packaging_sage_library[1]>", line 1
%timeit isinstance(sZZ, (Scheme, Pluffe)) # fast # random
^
SyntaxError: invalid syntax
**********************************************************************
File "src/doc/en/developer/packaging_sage_library.rst", line 343, in doc.en.developer.packaging_sage_library
Failed example:
%timeit isinstance(ZZ, (Scheme, Pluffe)) # fast # random
Exception raised:
Traceback (most recent call last):
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
compiled = compiler(example)
File "/srv/public/kliem/sage/local/var/lib/sage/venv-python3.7/lib/python3.7/site-packages/sage/doctest/forker.py", line 662, in compiler
compileflags, 1)
File "<doctest doc.en.developer.packaging_sage_library[2]>", line 1
%timeit isinstance(ZZ, (Scheme, Pluffe)) # fast # random
^
SyntaxError: invalid syntax
sagetrac-git commented
sagetrac-git commented
Branch pushed to git repo; I updated commit sha1. New commits:
7adef96 | src/doc/en/developer/packaging_sage_library.rst: Mark %timeit # not tested |
kliem commented
comment:16
Sorry, I completely forgot about this. I think figured to wait on a patchbot.
You know, you can also ping me. I might not react, but I won't be annoyed.
mkoeppe commented
comment:17
Thanks! There was no urgency
vbraun commented
Changed branch from u/mkoeppe/lazyimport___instancecheck______subclasscheck____return_false_on_importerror to 7adef96