feat: Allow resolving alias to external modules

[gilfree]

Hi

I would like to suggest adding an option (false by default) to set griffe's resolve external modules flag to true.

What are your thoughts about it? Did I do it correctly? As I am not a native English speaker, my documentation my need some improvement.

[pawamoy]

Hello, thanks for the PR! May I ask what is your use-case, and what you think of this comment mkdocstrings/mkdocstrings#503 (comment)?

[gilfree]

Hello, thanks for the PR! May I ask what is your use-case, and what you think of this comment mkdocstrings/mkdocstrings#503 (comment)?

Basically I have a rather large library that is exposed through multiple independent modules with other 'root' names - they do not share a package name (a bad case of need to preserve backward computability).

The use of __all__ is rather limited in this code base (And as I understand is discouraged in general in newer code as its goal is to allow * imports, which are discouraged). This means that I can easily control with __all__ what is pulled by griffe.

I think I can manage with pre loading as you suggested in the issue you have referenced, although it will require a bit more manual work, of finding out which modules to pre-load.

I think I can change this PR to add preload_modules: ['a','b','c'] option instead of load_external_mdules: bool. Would you prefer that I do this change?

From my POV the external and selectively using __all__ gives better ergonomics, but if you prefer explicit preloading, I will change the PR.

[pawamoy]

Basically I have a rather large library that is exposed through multiple independent modules with other 'root' names - they do not share a package name (a bad case of need to preserve backward computability).

I see, thanks.

The use of all is rather limited in this code base (And as I understand is discouraged in general in newer code as its goal is to allow * imports, which are discouraged). This means that I can easily control with all what is pulled by griffe.

Yeah, because of the lack of a standard way to "export" objects or make them public, __all__ is hijacked for this, while its original use is for wildcard imports, which are indeed heavily discouraged.

I think I can manage with pre loading as you suggested in the issue you have referenced, although it will require a bit more manual work, of finding out which modules to pre-load.

I think I can change this PR to add preload_modules: ['a','b','c'] option instead of load_external_mdules: bool. Would you prefer that I do this change?

From my POV the external and selectively using all gives better ergonomics, but if you prefer explicit preloading, I will change the PR.

Just to be clear: the preload solution would add a configuration option in the Python handler, and its value would be used as early as possible to load the specified modules in the handler's _module_collection. The workaround described in the comment I linked is... just a workaround.

Both (preload, and external+__all__) are compatible, and I don't mind having a configuration option to allow loading external modules, since you have a use-case for it, and I'm sure others will. I just didn't want to implement it myself (because I wouldn't use it), at least before the preload way is added. So if we are to merge this very PR, I'd like preload to be implemented first, as that would be the recommended way, while the external solution would be documented as "at your own risk".

I'm imagining something like this: starting from scratch, users temporarily allow loading external modules, and at the end of the build, the Python handler outputs an info/debug message suggesting to disable external loading and enable pre-loading with the list of external modules it loaded during the build. Maybe we could even try to exclude modules for which no objects were rendered from this suggested list. But these are nice-to-have, it could be done later.

If you feel like implementing both, that would be awesome! I'd recommend doing it in two distinct PRs. Let me know if that makes sense! In any case this conversation helped me clear my mind and I can see the path forward, so I'd eventually get it to it 🙂

[gilfree]

I changed the PR to preload modules. It works good enough for my use case, and I'm ok with that, and with adding external at a different PR.

Let me know if you think that is good enough.

[gilfree]

Hi.
I don't want to bother, but just in chance you missed the edits - do you think those (#60, #61) PRs are good enough now? Do you plan to accept them?
Thanks!

[pawamoy]

Hello! No worries, pinging me is the right way 🙂

[pawamoy]

OK, sounds good to me. I checked it out and it seems it's not possible to preload the modules in the __init__ method yet. So lets do it this way for now and maybe we'll refactor later 🙂 Thanks a lot!

[pawamoy]

LGTM, thanks!