autodoc and autosummary failing when class and module have the same name

larsoner commented 1 year ago

Describe the bug

We have mne/dipole.py in which there is a Dipole class defined. We import this class (and .dipole) in mne/__init__.py, and document with autosummary/autodoc the mne.Dipole class in the root namespace. On Windows (case-insensitive), we have the following problem, which just started show up on April 17th-ish in our builds:

C:\Users\larsoner\python\mne-python\mne\dipole.py:docstring of mne.dipole.Dipole:98: WARNING: autosummary: failed to import crop.
Possible hints:
* ValueError: not enough values to unpack (expected 2, got 1)
* ModuleNotFoundError: No module named 'crop'
* ModuleNotFoundError: No module named 'mne.Dipole.crop'; 'mne.Dipole' is not a package
* KeyError: 'crop'
* AttributeError: module 'mne' has no attribute 'crop'
* ModuleNotFoundError: No module named 'mne.crop'
* AttributeError: module 'mne.Dipole' has no attribute 'crop'

mne.Dipole.crop is a valid method and is documented properly on Linux. I think it's driven by the following behavior on Windows -- namely, that if you try import mne.Dipole Python imports mne/dipole.py module as mne.Dipole:

>>> import mne
>>> mne.Dipole   # correct!
<class 'mne.dipole.Dipole'>
>>> import mne.Dipole
>>> mne.Dipole  # bad!
<module 'mne.Dipole' from 'C:\\Users\\larsoner\\python\\mne-python\\mne\\Dipole.py'>
>>> from mne import Dipole  # bad enough, but continue...
>>> Dipole  # also bad!
<module 'mne.Dipole' from 'C:\\Users\\larsoner\\python\\mne-python\\mne\\Dipole.py'>

I think autosummary is doing some module importing under the hood here that is having this effect. I'm not sure if it's possible to directly try mne.dipole.Dipole or mne.Dipole before trying to import mne.Dipole, as that would fix the problem...?

How to Reproduce

I can try to come up with a MWE if it's not obvious that there is some existing workaround or what the problem is. I'm not sure it will be easy, though, as which errors we get for which classes appears to change run-to-run :(

Environment Information

At least sphinx 5.3 and the latest release (tested both).

Sphinx extensions

autodoc, autosummary

Additional context

I've looked at #4874 and changing the stub name does not help.
I've considered documenting mne.Dipole at mne.dipole.Dipole in the docs instead, but we really want this (and other classes with the same problem) in the root namespace. We also want mne.dipole to be public because some dipole-related functions live there and shouldn't live in the root mne namespace.

picnixz commented 1 year ago

The way autosummary is importing objects is using

https://github.com/sphinx-doc/sphinx/blob/3c4e78e2361b1e155fe7f6d29f1112c036d8e3b2/sphinx/ext/autosummary/__init__.py#L663-L707

Try to look inside the codesource directly

https://github.com/sphinx-doc/sphinx/blob/3c4e78e2361b1e155fe7f6d29f1112c036d8e3b2/sphinx/ext/autosummary/__init__.py#L316-L322

putting print here and there (I'm on Linux so it's kind of hard to reproduce the issue). First, it'd be good to know where the ValueError occurs because it's a bit weird. Also, it'd be good to know the original real target name that was tried.

What I understood:

The names that will be imported and checked are actually prefixed with some strings. This is done in https://github.com/sphinx-doc/sphinx/blob/3c4e78e2361b1e155fe7f6d29f1112c036d8e3b2/sphinx/ext/autosummary/__init__.py#L629
The prefixes that are used are constructed in https://github.com/sphinx-doc/sphinx/blob/3c4e78e2361b1e155fe7f6d29f1112c036d8e3b2/sphinx/ext/autosummary/__init__.py#L608. So it is worth taking a look at what is actually imported and in which order.

If you can tell me in the exact order of what was imported (and also why there was a ValueError), I can think of a workaround. By the way, the reason why autosummary_filename_map does not work is because it is used after the items are imported, so it will never be considered. The only idea I have is to actually bypass the import system and make it so that everything is imported in a case-sensitive way but 1) I don't have any testing environment 2) it might still be fragile

larsoner commented 1 year ago

1) I don't have any testing environment

FWIW nowadays you can get a Windows VM running for testing stuff. It would probably take ~1h to set it up with Python and everything, but it's at least doable if you're interested!

2) it might still be fragile

Indeed in thinking about the issue it's hard to see a solution that wouldn't be fragile given Python's own behavior of import mne.Dipole creating a mne.Dipole module :(

I'll try to look again on my Windows boot at some point but probably won't get to it for a bit.

sphinx-doc / sphinx