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

LazyImport.__instancecheck__, __subclasscheck__: Return False on ImportError #33017

Closed
mkoeppe opened this issue Dec 13, 2021 · 20 comments
Closed

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 13, 2021

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

CC: @tscrim @kliem

Component: refactoring

Author: Matthias Koeppe

Branch: 7adef96

Reviewer: Jonathan Kliem

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

@mkoeppe mkoeppe added this to the sage-9.5 milestone Dec 13, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 13, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 13, 2021

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 13, 2021

Commit: 447df69

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 13, 2021

New commits:

447df69LazyImport.__instancecheck__, __subclasscheck__: Return False on ImportError

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@kliem
Copy link
Contributor

kliem commented Dec 13, 2021

comment:5

It's not fast I suspect, as it tries an import and fails every time. But it is correct at least.

@kliem
Copy link
Contributor

kliem commented Dec 13, 2021

Reviewer: Jonathan Kliem

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 13, 2021

Dependencies: #32899

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 13, 2021

comment:8

I'll add some documentation to discuss performance

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 14, 2021

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

821d16esrc/doc/en/developer/packaging_sage_library.rst: Add bootstrap to testing instructions to avoid the trap of #32868
77c957fsrc/doc/en/developer/packaging_sage_library.rst: Put hierarchy section one level higher
9375ef8pkgs/sagemath-standard/tox.ini: Use SAGE_VENV or venv symlink to find wheels
c6fc111Prettier diagram
7d6a44cUse :mod: as markup for packages/modules
f83d424Improve ABC example
817a8d0src/doc/en/developer/packaging_sage_library.rst: More :mod: and :class: markup
b555735src/doc/en/developer/packaging_sage_library.rst: Link to pypi.org and to documentation of packaging metadata
fb151eaMerge #32899
907f8e8src/doc/en/developer/packaging_sage_library.rst: Expand on LazyImport, add timings

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 14, 2021

Changed commit from 447df69 to 907f8e8

@kliem
Copy link
Contributor

kliem commented Dec 15, 2021

comment:10

No patchbot yet, due to

https://groups.google.com/g/sage-devel/c/epajAbPvg6s/m/-yWW1gFyCAAJ

@kliem
Copy link
Contributor

kliem commented Dec 17, 2021

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
Copy link
Mannequin

sagetrac-git mannequin commented Dec 17, 2021

Changed commit from 907f8e8 to 7adef96

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 17, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

7adef96src/doc/en/developer/packaging_sage_library.rst: Mark %timeit # not tested

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 2, 2022
@kliem
Copy link
Contributor

kliem commented Apr 8, 2022

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
Copy link
Contributor Author

mkoeppe commented Apr 8, 2022

comment:17

Thanks! There was no urgency

@vbraun
Copy link
Member

vbraun commented Apr 10, 2022

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 10, 2022

Changed commit from 7adef96 to none

@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.6 Apr 10, 2022
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