Add caveats about lazy loading SPEC001

[tlambert03]

as requested by @jni over in scikit-image/scikit-image#6429, this adds a section about the caveats of lazy loading with regards to IDEs and static type checkers.

[stefanv]

Thanks, @tlambert03!

It strikes me that we have all the information available to generate classic imports. Would generating the stubs be helpful, or would it typically overwrite what is already there? Are there other ways in which we could guide the static type checkers without duplicating imports?

[tlambert03]

It strikes me that we have all the information available to generate classic imports. Would generating the stubs be helpful, or would it typically overwrite what is already there?

yep, for sure. I can imagine (and could take a stab if you'd like) a utility in lazy_loader that would generate these stubs on demand (we have something like this in napari but this one would be much simpler). The question then becomes when to "demand" it. You could do it at test time in CI, to detect whether a change has been made to public exports. (you could probably also add some sort of pre-commit thingy to do it automatically). Or, you could run some stub-generation script when generating your sdist/wheel. The choice between the two feels a bit subjective. thoughts?

Are there other ways in which we could guide the static type checkers without duplicating imports?

these are the only two I know of. I know that they both feel unfortunate in terms of duplication. I do generally implement some sort of automation/checking in my own projects, but I'm not sure I feel like there's an "obvious" one-size-fits all suggestion there

[stefanv]

There is one alternative, and that is for the lazy loader to have a utility function that parses a pyi to generate its own imports. Either way, not sure we can do this perfectly cleanly without built-in support as per that recent PEP.

[tlambert03]

and that is for the lazy loader to have a utility function that parses a pyi to generate its own imports.

good point! how about this approach? feels kinda clean and seems to work fine for (e.g.) skimage.filters

# skimage/filters/__init__.py
from .._shared import lazy

__getattr__, __dir__, __all__ = lazy.attach_stub(__name__, __file__)

# skimage/_shared/lazy.py
...

import ast

class Visitor(ast.NodeVisitor):
    def __init__(self) -> None:
        self._submodules = set()
        self._submod_attrs = {}

    def visit_ImportFrom(self, node: ast.ImportFrom):
        assert node.level == 1, 'Currently only support `import from .*`'
        if node.module:
            attrs: list = self._submod_attrs.setdefault(node.module, [])
            attrs.extend(alias.name for alias in node.names)
        else:
            self._submodules.update(alias.name for alias in node.names)


def attach_stub(package_name: str, filename: str):
    stubfile = f'{filename}i'

    if not os.path.exists(stubfile):
        raise ValueError(f"Stub file {stubfile} does not exist")

    with open(stubfile) as f:
        stub_node = ast.parse(f.read())

    visitor = Visitor()
    visitor.visit(stub_node)
    return attach(package_name, visitor._submodules, visitor._submod_attrs)

# skimage/filters/__init__.pyi

from . import rank
from ._fft_based import butterworth
from ._gabor import gabor, gabor_kernel
from ._gaussian import difference_of_gaussians, gaussian
from ._median import median
from ._rank_order import rank_order
from ._sparse import correlate_sparse
from ._unsharp_mask import unsharp_mask
from ._window import window
from .edges import (farid, farid_h, farid_v, laplace, prewitt, prewitt_h,
                    prewitt_v, roberts, roberts_neg_diag, roberts_pos_diag,
                    scharr, scharr_h, scharr_v, sobel, sobel_h, sobel_v)
from .lpi_filter import LPIFilter2D, inverse, wiener
from .ridges import frangi, hessian, meijering, sato
from .thresholding import (apply_hysteresis_threshold, threshold_isodata,
                           threshold_li, threshold_local, threshold_mean,
                           threshold_minimum, threshold_multiotsu,
                           threshold_niblack, threshold_otsu,
                           threshold_sauvola, threshold_triangle,
                           threshold_yen, try_all_threshold)

[jni]

I love that @tlambert03 😍

[stefanv]

@tlambert03 I see the PR I requested already exists 😍 Now the your lazy stubs PR has been merged, we can modify this one and get it in as well. Thanks again!

[tlambert03]

updated! lemme know if you see anything missing, or would like anything changed

[stefanv]

@tlambert03 Sorry for the delay, I was on parental leave. But now I'm back :)

I worked a bit on your PR to make it shorter, and to make the advice more direct (if opinionated). Please take a look at tlambert03#1 and let me know what you think?

[stefanv]

Thanks again for your work on the stubs loader!

[stefanv]

@tlambert03 It looks like this trick doesn't quite fool pyright, which complains that the modules are not actually imported.

[tlambert03]

Yes, I think there's no getting around a static, manually entered __all__ in that case. This approach isn't incompatible with that, but that would be an additional step. (And maybe still not even work... not sure)