Handling path identifiers

[vinRosso]

Discussed in #105

Following mkdocstrings/griffe#158

---

Premises

Based on discussion on mkdocstrings/griffe#158 I ended up making my custom handler based on this repository.

The purpose is to allow handling of path identifiers like
::: photon/ph_diskconnection/1.6.0/ph_diskconnection.managers.usd.assemblymanager

The original issue is that our pipeline is dividing submodules per version, since we are using AcademySoftwareFoundation/rez that allows us to build different environments based on the project we're working on.
Obviously I can't use the identifier
::: photon.ph_diskconnection.1.6.0.ph_diskconnection.managers.usd.assemblymanager

---

Issue

Having our own handler implies that we need to keep it updated with the main branch from this repository and it is quite time consuming.

So I thought that adding the changes from our custom handler to this repo would make a nice additional feature that maybe other people could use.

---

Solution

The solution is simple, it is based on recognizing wether the given identifier is a path and splitting the package path from the actual identifier. Also, if the module identifier comes from a path, it is treated as an unknown module.

# handler.py

def collect(self, identifier: str, config: Mapping[str, Any]) -> CollectorItem: 
+   # check if identifier is a path and separate absolute path from identifier
+   os_sep = os.path.sep
+   if os_sep in identifier:
+       module_path, identifier = identifier.rsplit(os_sep, 1)
+   else:
+       module_path = ""

    module_name = identifier.split(".", 1)[0]
    unknown_module = module_name not in self._modules_collection
    if config.get("fallback", False) and unknown_module:
        raise CollectionError("Not loading additional modules during fallback")

    # See: https://github.com/python/typeshed/issues/8430
    mutable_config = dict(copy.deepcopy(config))
    final_config = ChainMap(mutable_config, self.default_config)
    parser_name = final_config["docstring_style"]
    parser_options = final_config["docstring_options"]
    parser = parser_name and Parser(parser_name)

+   if unknown_module or module_path:  # if identifier is a path, suppose it's a new module
        loader = GriffeLoader(
            extensions=load_extensions(final_config.get("extensions", [])),
+           search_paths=[*self._paths, module_path],
            docstring_parser=parser,
            docstring_options=parser_options,
            modules_collection=self._modules_collection,
            lines_collection=self._lines_collection,
            allow_inspection=final_config["allow_inspection"],
        )

The test for this special case would be very generic, where we could just provide whatever path to a python module.

# test_handler.py

def test_collect_module_from_path() -> None:
    """Assert existing module can be collected from path."""
    handler = get_handler(theme="material")
    full_path = __file__.rsplit(".", 1)[0]
    assert handler.collect(full_path, {})

[pawamoy]

Thanks 🙂

So, I would be OK with the change if there wasn't an issue with it: doing this means we allow collecting and rendering docs for the same module multiple times (the different versions of it), and this is not compatible with how our object inventories work. Cross-references and inter-links are a pretty important feature of code documentation tools, and I don't want to allow changes that will generate issues in the future.

Of course, if we can find a way to mitigate the issue, I'd be considering this feature with more interest 🙂

A few initial thoughts:

• we can't use a single inventory anymore
• so maybe we could add a way to provide multiple inventories
• it means we'd have to tell mkdocstrings where to write each of these inventories
• the only way I see to do this would be to pass the info from HTML to mkdocstrings extension, using a data-inventory attribute maybe
• in turn, the HTML attribute would be set by the templates, so by the render method of the handler
• in turn, the render method would need to be passed this option somehow (local option in YAML blocks?)

::: path/to/package.module.Class
    options:
      inventory: /url/to/inventory

I'd like to see the layout of your built docs, to see where each inventory could be put.h, and if this proposal makes any sense.

---

You see that that is quite challenging to get right, so unfortunately I believe you'll have to maintain your custom handler for a bit longer 😕

You mentioned keeping updated with the main branch: do I guess correctly that you're using a fork? Another solution would be to depend on the handler (no forks), subclassing it and overriding the collect method only. As long as the interface doesn't change, you don't have to do anything anymore: users will pick up the latest mkdocstrings-python when installing your custom handler package. They would have to change the default_handler in mkdocs.yml though.

[vinRosso]

Yeah, cross-references and inter-links are broken in our documentation when referencing another package, but since our packages are almost all independent we never really saw that issue. But I can see why it would be important to keep them working. In that sense I can't see any solution unfortunately.

For the inventory I didn't exactly understand how it's used, since I couldn't find any references in the python handler. I suppose it's managed from mkdocstrings? In any case we could make it use different inventories based on the root path instead of passing the info (it's supposed that all the modules from a version share the same root path), but without a solution for the cross-references that would be useless. :sad

---

Thanks for the suggestions, I think I'll go with the subclass solution, it sounds like the most efficient. I didn't think about it but it could be the best way to go given our situation.

For now I think you can close this issue, unless you don't have a mind-blowing idea for the cross-references hahaha.
Thanks for your help! :)

[pawamoy]

Thanks for your feedback! I will close this then, while keeping in mind that some users have this need 🙂