fix: support editable install with multiple roots

[gilfree]

Python packages may have multiple root modules where each is installed to its own folder under site-packages.

When using new setuptools editable install in such cases griffe, when used by mkdocstrings fails to find the correct paths.

The root cause it that griffe assumed a variable named MAPPING in the .py file created by the editable install, and assumed that this variable is a dict with single entry.

When a package has multiple roots - then this dictionary contain multiple entries.

This PR aims to handle such cases.

I have no knowledge about editables so I could not fix that case.

The unit test added simulates the case, and this has been tested against a real complex package with multiple roots using setuptools 65 editable install.

[gilfree]

Hi. 😄

My first attempt in PR here.
I hope I was able to properly follow the contributing guide.
If there is something that needs to be changed - let me know.

[pawamoy]

Thanks, that's great! You did very well for your first contribution here 🚀

I have a few suggestions below 🙂

[pawamoy]

At this point I feel like we should parse the entire file with AST (without any evaluation), and iterate on the module's body to find the MAPPING assignment node. Who knows if setuptools won't split the assignment on multiple lines in some cases, or in the future?

It would look like this:

parsed_module = ast.parse(path.read_text())
for node in parsed_module.body:
    if isinstance(node, ast.Assign) and node.targets[0].id == "MAPPING":
        return [Path(constant.value).parent for constant in node.value.values]

[gilfree]

I have changed to full AST parsing, I agree it will be more stable.

A concern I do have at this point is that the change by setuptools or editables or any other package will be bigger than that.

It is probably not for this MR, and I don't understand the ModuleFinder mechanism enough, but I think it might probably be augmented by using the sys.meta_hook, and in that case there is no need to specially handle each editable install variant.

If you prefer, I think the code below may replace the _extend_from_editable_modules

    def find_package(self, module_name: str) -> Package | NamespacePackage:  # noqa: WPS231
        """Find a package or namespace package.

        Parameters:
            module_name: The module name.

        Raises:
            ModuleNotFoundError: When the module cannot be found.

        Returns:
            A package or namespace package wrapper.
        """
        filepaths = [
            Path(module_name),
            # TODO: handle .py[cod] and .so files?
            Path(f"{module_name}.py"),
        ]
       # Add meta path
        specs_paths = []
        for finder in sys.meta_path:
            spec = finder.find_spec(module_name,None)
            if spec and spec.submodule_search_locations:
                specs_paths.extend(Path(path).parent for path in spec.submodule_search_locations)
        namespace_dirs = []
        for path in self.search_paths + specs_paths:  # noqa: WPS440
            path_contents = self._contents(path)
            if path_contents:
                for choice in filepaths:
                    abs_path = path / choice
                    if abs_path in path_contents:
                        if abs_path.suffix:
                            stubs = abs_path.with_suffix(".pyi")
                            return Package(module_name, abs_path, stubs if stubs.exists() else None)
                        else:
                            init_module = abs_path / "__init__.py"
                            if init_module.exists() and not _is_pkg_style_namespace(init_module):
                                stubs = init_module.with_suffix(".pyi")
                                return Package(module_name, init_module, stubs if stubs.exists() else None)
                            namespace_dirs.append(abs_path)

        if namespace_dirs:
            return NamespacePackage(module_name, namespace_dirs)

        raise ModuleNotFoundError(module_name)

[pawamoy]

Thanks! For reference, where did you get that code from?

I am indeed thinking about replacing Griffe's whole package finding algorithm with a subclass of what the standard lib offers, but just like you, I don't understand it enough and was not able to conclude if it would fit Griffe's needs or not. But I'm definitely open to suggestions as that would drastically simplify the code base.

[gilfree]

This is a small change on the griffe code - just added the lines below (which I wrote myself) to the function from griffe, and iterated also on specs_paths

        specs_paths = []
        for finder in sys.meta_path:
            spec = finder.find_spec(module_name,None)
            if spec and spec.submodule_search_locations:
                specs_paths.extend(Path(path).parent for path in spec.submodule_search_locations)

[gilfree]

I have tried to fix according to your comments.
Thanks.

[gilfree]

I have changed to full AST parsing, I agree it will be more stable.

A concern I do have at this point is that the change by setuptools or editables or any other package will be bigger than that.

It is probably not for this MR, and I don't understand the ModuleFinder mechanism enough, but I think it might probably be augmented by using the sys.meta_hook, and in that case there is no need to specially handle each editable install variant.

If you prefer, I think the code below may replace the _extend_from_editable_modules

    def find_package(self, module_name: str) -> Package | NamespacePackage:  # noqa: WPS231
        """Find a package or namespace package.

        Parameters:
            module_name: The module name.

        Raises:
            ModuleNotFoundError: When the module cannot be found.

        Returns:
            A package or namespace package wrapper.
        """
        filepaths = [
            Path(module_name),
            # TODO: handle .py[cod] and .so files?
            Path(f"{module_name}.py"),
        ]
       # Add meta path
        specs_paths = []
        for finder in sys.meta_path:
            spec = finder.find_spec(module_name,None)
            if spec and spec.submodule_search_locations:
                specs_paths.extend(Path(path).parent for path in spec.submodule_search_locations)
        namespace_dirs = []
        for path in self.search_paths + specs_paths:  # noqa: WPS440
            path_contents = self._contents(path)
            if path_contents:
                for choice in filepaths:
                    abs_path = path / choice
                    if abs_path in path_contents:
                        if abs_path.suffix:
                            stubs = abs_path.with_suffix(".pyi")
                            return Package(module_name, abs_path, stubs if stubs.exists() else None)
                        else:
                            init_module = abs_path / "__init__.py"
                            if init_module.exists() and not _is_pkg_style_namespace(init_module):
                                stubs = init_module.with_suffix(".pyi")
                                return Package(module_name, init_module, stubs if stubs.exists() else None)
                            namespace_dirs.append(abs_path)

        if namespace_dirs:
            return NamespacePackage(module_name, namespace_dirs)

        raise ModuleNotFoundError(module_name)

[pawamoy]

OK, LGTM, thanks so much 🚀