Skip to content

Use lazy.attach_stub for lazy imports, to accommodate type checkers and IDEs#6435

Closed
tlambert03 wants to merge 8 commits intoscikit-image:mainfrom
tlambert03:lazy-stub
Closed

Use lazy.attach_stub for lazy imports, to accommodate type checkers and IDEs#6435
tlambert03 wants to merge 8 commits intoscikit-image:mainfrom
tlambert03:lazy-stub

Conversation

@tlambert03
Copy link
Copy Markdown
Contributor

@tlambert03 tlambert03 commented Jul 12, 2022

Description

This is an alternative to #6429 that uses pyi type stubs instead of an if TYPE_CHECKING clause. At runtime, when a module is imported, it will look for an adjacent .pyi, parse the abstract syntax tree, and pass the detected modules and sumodule attrs to the regular lazy.attach() function. The benefits here are that:

  1. type checkers and IDEs can be happy again, while retaining lazy imports
  2. no code desynchronization between dynamic exports and typing exports
  3. you get to declare exports in standard from *. import syntax.
  4. (compared to add TYPE_CHECKING hints for lazy imports in data and filters #6429), stylistically you get to avoid if TYPE_CHECKING

Some caveats to be aware of:

  1. the stub files must be distributed in the sdist/wheel, or this will fail (I've added a line to setup.py here to that effect)
  2. As with lazy.attach(), this currently only supports within-module exports. Meaning you can't use a relative import that goes beyond the module from ..something import. lazy.attach also doesn't support this, but it's a bit easier to accidentally do it with a type stub since the syntax is more familiar. In my implementation, attempting this will raise an exception at runtime, so it will be easy to catch with tests.3.
  3. the __all__ clauses are unfortunately required because type checkings treat stub imports as implicit: https://mypy.readthedocs.io/en/stable/config_file.html#confval-implicit_reexport. It would still "work" without the __all__ clause, but you would get no suggestions when you do from skimage.filters import ...

(this pattern was also added as a PR to lazy_loader: scientific-python/lazy-loader#10 and is vendored here)

Checklist

  • Docstrings for all functions
  • Gallery example in ./doc/examples (new features only)
  • Benchmark in ./benchmarks, if your changes aren't covered by an
    existing benchmark
  • Unit tests
  • Clean style in the spirit of PEP8
  • Descriptive commit messages (see below)

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.
  • There is a bot to help automate backporting a PR to an older branch. For
    example, to backport to v0.19.x after merging, add the following in a PR
    comment: @meeseeksdev backport to v0.19.x
  • To run benchmarks on a PR, add the run-benchmark label. To rerun, the label
    can be removed and then added again. The benchmark output can be checked in
    the "Actions" tab.

@tlambert03 tlambert03 changed the title Lazy stub Use lazy.attach_stub for lazy imports, to accommodate type checkers and IDEs Jul 12, 2022
@jni
Copy link
Copy Markdown
Member

jni commented Jul 13, 2022

One thing I hadn't quite considered is that this is now a semi-clean way to enforce that the module-level API is the public API, and things not in the stub will work but be hard to find, right?

@tlambert03
Copy link
Copy Markdown
Contributor Author

Yes.... but, I'm not sure I'm quite following your thought: are you contrasting that with the previous setup?

@alexdesiqueira
Copy link
Copy Markdown
Member

Thank you @tlambert03! @jni are you still happy with this one? I'll merge if you are.
Thanks both!

@stefanv
Copy link
Copy Markdown
Member

stefanv commented Jan 9, 2023

Implemented in #6577 using lazy_loader.attach_stub, implemented by @tlambert03

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants