Autosummary: `autosummary_ignore_module_all = False` changes behavior of `:recursive:`.

howieyoo commented 8 months ago

Describe the bug

Autosummary's recursive option enables generation of documentation stubs for sub-package and modules. When setting autosummary_ignore_module_all to False, the behavior of the recursive option changes such that it only generates documentation stubs for modules which are imported in to the module currently being generated.

How to Reproduce

Setup

Package structure:

package/
    __init__.py
    submodule.py
    subpackage/
        __init__.py
        subpackagemodule.py

Contents of package/__init__.py:

import os as alias0
import sys as alias1

In conf.py:

extensions = ["sphinx.ext.autodoc", "sphinx.ext.autosummary"]
autosummary_ignore_module_all = True  # default

In index.rst:

.. autosummary::
   :toctree: api
   :recursive:

   package

Generating the documentation produces the following stubs:

api/
    package.rst
    package.submodule.rst
    package.subpackage.rst
    package.subpackage.subpackagemodule.rst

Reproduction

Disable autosummary_ignore_module_all in conf.py.
```
autosummary_ignore_module_all = False
```
Define an __all__ variable in package/__init__.py.
```
__all__ = ["alias0"]
```
Delete contents of api.
Generate the documentation.
```
api/
package.rst
package.alias0.rst
```

The config is working as documented, but note that the sub-package and modules are no longer generated.

Environment Information

Platform:              linux; (Linux-6.5.0-17-generic-x86_64-with-glibc2.35)
Python version:        3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0])
Python implementation: CPython
Sphinx version:        7.2.6
Docutils version:      0.20.1
Jinja2 version:        3.1.3
Pygments version:      2.17.2

Sphinx extensions

extensions = ["sphinx.ext.autodoc", "sphinx.ext.autosummary"]

Additional context

No response

jayaddison commented 8 months ago

Hey @howieyoo - thanks for writing this up.

As far as I can tell, it seems to be working as intended -- the __all__ = [ ... ] entry in your module is an indication of what should be available in the module (to import from elsewhere), and the autosummary extension is recursing through those - and only those - when autosummary_ignore_module_all = False.

From experimenting with this locally, adding import package.subpackage as subpackage and appending subpackage to the __all__ entries does allow autosummary to recurse into there.

And conversely, when autosummary_ignore_module_all = True, the recursion happens without __all__ being considered.

Is there a bug you've found somewhere here, or is this OK as-is?

howieyoo commented 8 months ago

The way I see it, there is a difference between import module.submodule when submodule is a separate module (it's own .py file) and when submodule is a member of module which happens to be of type ModuleType. When autosummary_ignore_module_all = True, recursive autosummary includes the former. When autosummary_ignore_module_all = False and __all__ is defined, it includes the latter.

From experimenting with this locally, adding import package.subpackage as subpackage and appending subpackage to the all entries does allow autosummary to recurse into there.

Yes, that will result in the desired behavior w.r.t to documentation, but exporting your submodules via __all__ changes the semantics of the module. If you were to do this in order to get recursive documentation generation your submodules would be pulled in to the namespace as well. I would say most of the time this is not the behavior you want.

According to #6040 the spirit of :recursive: was to discover sub-modules by walking the file systems (via iter_modules) so you didn't need to manually add them all in your autosummary directive. In my opinion, autosummary_ignore_module_all breaks this feature (the bug). Furthermore, if you modify the autosummary recursive test case by adding autosummary_ignore_module_all = False to conf.py and add __all__ = [] to package/__init__.py it will fail.

My expectation was that :recursive: always generated stubs for modules discovered through iter_modules, regardless of how autosummary is configured to document the contents of the module.

I would like to suggest that :recursive: always include sub-modules found via iter_modules which provides the convenience factor of not having to list your modules in an autosummary directive and not having to export your modules in __all__. Have autosummary_ignore_module_all control which members of the module are documented. Including members of type ModuleType. Then generate module stubs for the union of submodules and members of type ModuleType.

jayaddison commented 8 months ago

Thank you for the explanation.

I would like to suggest that :recursive: always include sub-modules found via iter_modules which provides the convenience factor of not having to list your modules in an autosummary directive and not having to export your modules in __all__. Have autosummary_ignore_module_all control which members of the module are documented. Including members of type ModuleType. Then generate module stubs for the union of submodules and members of type ModuleType.

This sounds reasonable to me. Would you be willing to offer a pull request to adapt the existing behaviour? I think that modifying/extending the test-ext-autosummary-* test cases would be the place to start.

howieyoo commented 8 months ago

Sure. I'll give this a go next time I have a look at documenting my project.

sphinx-doc / sphinx