Make lazy_import more friendly to pyflakes and other static checkers

[mkoeppe]

pyflakes does not know about lazy_import and may therefore emit distracting warnings - as noted in #30616 comment:8

We introduce a new method PythonModule.lazy_import, which enables the following new idiom for creating lazy imports that are visible to static checkers:

LibBraiding = PythonModule('sage.libs.braiding', spkg='libbraiding')
rightnormalform = LibBraiding.lazy_import(
  'rightnormalform', namespace=None)
centralizer,  supersummitset = LibBraiding.lazy_import(
  ('centralizer', 'supersummitset'), namespace=None)

See also:

• 22793 New LazyImport implementation based on lazy_object_proxy

• 25898 Enable setting attributes on a lazy imported object

Depends on #30616

CC: @dcoudert @tobiasdiez @fchapoton @jhpalmieri @tscrim

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: u/mkoeppe/make_lazy_import_more_friendly_to_pyflakes_and_other_static_checkers @ 9b19c7a

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

[mkoeppe]

Description changed:

--- 
+++ 
@@ -3,6 +3,8 @@
 It should be investigated whether it is possible to support a syntax for lazy imports that looks like `import` to pyflakes, using Python 3's import machinery.

 See also: 
-- #22793 New LazyImport implementation based on lazy_object_proxy
+- #22793 New `LazyImport` implementation based on `lazy_object_proxy`
+- #25898 Enable setting attributes on a lazy imported object

+

[tobiasdiez]

comment:2

As a workaround, you could use lazy imports in conjunction with traditional imports, but place the traditional imports within a if TYPE_CHECKING: statement. That way, it won't execute at runtime, but the type checker will know about import for type checking purposes.

(from https://github.com/microsoft/pyright/issues/954#issuecomment-675516511)

[fchapoton]

comment:3

Note that the patchbot "pyflakes plugin" knows about lazy imports.

BUT only with a basic syntax, one-liners.

[mkoeppe]

comment:4

Ah, OK, found it at https://github.com/sagemath/sage-patchbot/blob/master/sage_patchbot/plugins.py#L283

[mkoeppe]

comment:5

Replying to @tobiasdiez:

As a workaround, you could use lazy imports in conjunction with traditional imports, but place the traditional imports within a if TYPE_CHECKING: statement. That way, it won't execute at runtime, but the type checker will know about import for type checking purposes.

(from https://github.com/microsoft/pyright/issues/954#issuecomment-675516511)

Thanks for the pointer!

Given that lazy imports are so widely used in sage, we probably don't want to use this workaround unless really necessary. It would add a lot of clutter to lots of files

[mkoeppe]

comment:6

A simple solution would be to make lazy_import return the (tuple of) lazy import objects that it creates, instead of relying on the magical namespace injection as a side effect. Then we could write

rightnormalform, centralizer, supersummitset = lazy_import(
  'sage.libs.braiding',
  ['rightnormalform', 'centralizer', 'supersummitset'],
  feature=PythonModule('sage.libs.braiding', spkg='libbraiding'))

Of course, we could as well just use LazyImport directly:

LibBraiding = PythonModule('sage.libs.braiding', spkg='libbraiding')
rightnormalform = LazyImport('sage.libs.braiding', 'rightnormalform', feature=LibBraiding)
centralizer     = LazyImport('sage.libs.braiding', 'centralizer',     feature=LibBraiding)
supersummitset  = LazyImport('sage.libs.braiding', 'supersummitset',  feature=LibBraiding)

[mkoeppe]

comment:7

Expanding on this, we could introduce a method PythonModule.lazy_import that would enable writing this:

LibBraiding = PythonModule('sage.libs.braiding', spkg='libbraiding')
rightnormalform = LibBraiding.lazy_import('rightnormalform')
centralizer,  supersummitset = LibBraiding.lazy_import(
  'centralizer', 'supersummitset')

[tobiasdiez]

comment:8

I think this will not be sufficient for the static typing analysis. You would need to provide the information that lazy_import('class') returns an instance of class.

Maybe one could declare it as lazy_import(Type[C] cls) -> C and call it via lazy_import(sage.libs.braiding.rightnormalform) (note that the argument is not a string). Not sure if that works.

[mkoeppe]

Branch: u/mkoeppe/make_lazy_import_more_friendly_to_pyflakes_and_other_static_checkers

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

81207ae

lazy_import: Use namespace=True as the default; if namespace=None, return the LazyImport objects

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Commit: 81207ae

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 81207ae to 8922351

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

4793555	sage.features.PythonModule.lazy_import: New
8922351	src/sage/graphs/graph.py: Use PythonModule.lazy_import

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

9b19c7a

src/sage/groups/braid.py: Use PythonModule.lazy_import

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 8922351 to 9b19c7a

[mkoeppe]

comment:13

Here's something that seems to work.

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

comment:14

Replying to @tobiasdiez:

I think this will not be sufficient for the static typing analysis. You would need to provide the information that lazy_import('class') returns an instance of class.

In the current version, I suppose one could just use a typed assignment

[mkoeppe]

Description changed:

--- 
+++ 
@@ -1,10 +1,16 @@
 `pyflakes` does not know about `lazy_import` and may therefore emit distracting warnings - as noted in [#30616 comment:8](https://github.com/sagemath/sage/issues/30616#comment:8)

-It should be investigated whether it is possible to support a syntax for lazy imports that looks like `import` to pyflakes, using Python 3's import machinery.
+We introduce a new method ``PythonModule.lazy_import``, which enables the following new idiom for lazy imports that are visible to static checkers:
+
+```
+LibBraiding = PythonModule('sage.libs.braiding', spkg='libbraiding')
+rightnormalform = LibBraiding.lazy_import(
+  'rightnormalform', namespace=None)
+centralizer,  supersummitset = LibBraiding.lazy_import(
+  ('centralizer', 'supersummitset'), namespace=None)
+```

 See also: 
 - #22793 New `LazyImport` implementation based on `lazy_object_proxy`
 - #25898 Enable setting attributes on a lazy imported object

-
-

[mkoeppe]

Description changed:

--- 
+++ 
@@ -1,6 +1,6 @@
 `pyflakes` does not know about `lazy_import` and may therefore emit distracting warnings - as noted in [#30616 comment:8](https://github.com/sagemath/sage/issues/30616#comment:8)

-We introduce a new method ``PythonModule.lazy_import``, which enables the following new idiom for lazy imports that are visible to static checkers:
+We introduce a new method `PythonModule.lazy_import`, which enables the following new idiom for creating lazy imports that are visible to static checkers:

LibBraiding = PythonModule('sage.libs.braiding', spkg='libbraiding')

[mkoeppe]

comment:18

I was hoping for a syntax such as from sage.__lazy__.libs.braiding import rightnormalform. I think this can be implemented with the python3 import machinery (https://docs.python.org/3/reference/import.html#finders-and-loaders) but haven't investigated.

[mkoeppe]

Dependencies: #30616

[jhpalmieri]

comment:20

Could you please document the feature argument in lazy_import.pyx?

[tobiasdiez]

comment:21

Replying to @mkoeppe:

Replying to @tobiasdiez:

I think this will not be sufficient for the static typing analysis. You would need to provide the information that lazy_import('class') returns an instance of class.

In the current version, I suppose one could just use a typed assignment

What do you mean with typed assignment? In the previous version,

try:
    from sage.graphs.mcqd import mcqd
except ImportError:
    from sage.misc.package import PackageNotFoundError
    raise PackageNotFoundError("mcqd")
return mcqd(self)

static type checkers can check if mcqd is actually a function defined in sage.graphs.mcqd and if self is allowed as an argument of mcqd. I doubt that they are intelligent enough to give the same information for the new version:

from sage.features import PythonModule
Mcqd = PythonModule('sage.graphs.mcqd', spkg='mcqd')
mcqd = Mcqd.lazy_import('mcqd', namespace=None)
return mcqd(self)

[tobiasdiez]

comment:23

Another option would be to introduce (global) booleans that indicate whether a given library is available. E.g.

// e.g. in sage.distribution
def is_mcqd():
   try:
      from sage.graphs.mcqd import mcqd
      return true 
   except ImportError:
      return false
   // or some other way to determine if mcqd is available

// usage in another module:

// at the beginning: global import
if sage.distribution.is_mcqd():
    from sage.graphs.mcqd import mcqd
...
// later in the code
if sage.distribution.is_mcqd():
    return mcqd(self)
else:
    return ...

Yet another solution would be the following pattern:

try:
    from sage.graphs.mcqd import mcqd as _mcqd 
except ImportError:
    mcqd = None
else:
    mcqd = _mcqd

// later
if mcqd:
    return mcqd(self)
else:
    return ...

suggested in https://github.com/python/mypy/issues/1297#issuecomment-508593494. This should work with mypy, not sure about pyright.

[mkoeppe]

comment:24

Replying to @jhpalmieri:

Could you please document the feature argument in lazy_import.pyx?

This has now happened in #30587.

[mkoeppe]

comment:25

Replying to @mkoeppe:

I was hoping for a syntax such as from sage.__lazy__.libs.braiding import rightnormalform. I think this can be implemented with the python3 import machinery (https://docs.python.org/3/reference/import.html#finders-and-loaders) but haven't investigated.

This was previously discussed in #22752

[mkoeppe]

comment:26

Another solution is to provide type stub files (__init__.pyi), see section "type checkers" in https://scientific-python.org/specs/spec-0001/