fix: proper error messages if autosummary fails to import module

[Pro]

Subject: Show proper error messages if autosummary fails to import module

Fixes #7989

Feature or Bugfix

• Bugfix

Purpose

As described in #7989, autosummary currently does not provide helpful error messages.

E.g., let's say I have a module called fancy_bob, which in its init.py uses import filelock (or any other external module).
Then in my docs I write:

.. autosummary::
   :toctree: _autosummary
   :recursive:

   fancy_bob

If the filelock module is not installed, autosummary will just fail with:

[autosummary] failed to import 'fancy_bob': no module named fancy_bob.

This error message is completely misleading, since it actually finds the module fancy_bob but then fails to import its dependencies.

This MR changes the error output in a way that you at least get some hints what went wrong, e.g., now you get:

[autosummary] failed to import 'fancy_bob': no module named fancy_bob. Possible hints: ["No module named 'filelock'"]

Or if there is a syntax error in the module's init.py, it will show:

[autosummary] failed to import 'fancy_bob': no module named fancy_bob. Possible hints: ['invalid syntax (__init__.py, line 4)']

Reproduction

@astrojuanlu provided a nice example project to reproduce the issue in https://github.com/astrojuanlu/sphinx-autosummary-tracebacks

Not setting the pythonpath properly results in:

[autosummary] failed to import 'test_pkg.A': no module named test_pkg.A. Possible hints: ["No module named 'test_pkg'", 'no module named test_pkg']

Setting the pythonpath gives the correct error:

[autosummary] failed to import 'test_pkg.A': no module named test_pkg.A. Possible hints: ['no module named test_pkg', "No module named 'attr'"]

I.e., "No module named 'attr'

Detail

• autosummary doesn’t report correct module on ImportError #7989

Relates

[AndiEcker]

+1 thank you very much @Pro

[flying-sheep]

Your stringification is a bit wonky, but generally good idea!

[tk0miya]

I feel this change is strange because last_errors should be a part of the return value. I understand using argument gives us compatibility. But...

BTW, it should be annotated.

[Pro]

Also thought the same, but that would probably break backwards compatibility? Or should I change last_errors to be a retun value?

[tk0miya]

I have two ideas:

1. Add another version of import_by_name that also returns list of errors. It will be named as different name.
2. Add a boolean argument like raise_error to import_by_name. And it raises a custom exception that contains list of errors.

[Pro]

Hmm, raise_error would not work that well since we need to collect all errors during the various import methods there are.

Having another version of the import_by_name is IMHO also not such an ideal solution, since then we would need to duplicate code, not only for the import_by_name but also for the sub-methods such as _import_by_name which is called inside of import_by_name

[tk0miya]

I have two ideas:

1. Add another version of import_by_name that also returns list of errors. It will be named as different name.
2. Add a boolean argument like raise_error to import_by_name. And it raises a custom exception that contains list of errors.

[Pro]

@tk0miya is there still something missing to merge this MR?

[tk0miya]

Add a boolean argument like raise_error to import_by_name. And it raises a custom exception that contains list of errors.

I just post #10031 to implement my idea.

[Pro]

Add a boolean argument like raise_error to import_by_name. And it raises a custom exception that contains list of errors.

I just post #10031 to implement my idea.

Cool! Also nice solution!
I will close this MR in favor of yours.