Repository: sphinx-doc/sphinx

Issue:
Autosummary: ``autosummary_ignore_module_all`` does not work as documented
### Describe the bug

As per documentation :

> Note that if an imported member is listed in `__all__`, it will be documented regardless of the value of autosummary_imported_members.

So the way I understand it is that if I have a small package like this :

```
my_package/
    __init__.py
    my_class.py
    my_module/
        my_submodule.py
```

with the following files :

`my_class.py`
```python
class myClass:
    """" my Class docstring """"
    def my_class_method(self, a):
        """ method docstring """
        return
```

`my_submodule.py`
```python
def f():
    """ f docstring """
    return
```

`__init__.py`
```python
from .my_class import myClass
from . import my_module

__all__ = ["myClass", "my_module"]
```

and a rst file like this:

```rst
An example of recursive module documentation
====================================

.. autosummary::
    :toctree: generated
    :recursive:

    my_package
```

assume the option `autosummary_ignore_module_all` is set to `False` in `conf.py`, the autosummary should only consider `myClass` and `my_module` to be part of the package `my_package`. The `generated` folder should normally look like this:

```
generated
    my_package.myClass
    my_packge.my_module
```

But actually, it looks like this :

```
generated
    my_package.my_class
```

Meaning it ignored the imported elements, even though they were in the `__all__` and it did take the `my_module` package even though it was not in the `__all__`



### How to Reproduce

```
$ git clone git@github.com:ClementPinard/sphinx_autosummary_bug_example.git
$ cd sphinx_autosummary_bug_example.gt
$ pip install sphinx
$ sphinx-build -b html docs docs/_build
$ # see docs/generated/ folder
```


### Expected behavior

generated folder should have the files

```
my_package.rst
my_package.my_module.rst
my_patckage.my_module.my_submodule.rst
my_package.myClass.rst
```


### Your project

https://github.com/ClementPinard/sphinx_autosummary_bug_example

### Screenshots

_No response_

### OS

Ubuntu 20.04

### Python version

3.9

### Sphinx version

5.1

### Sphinx extensions

sphinx.ext.autodoc, sphinx.ext.autosummary

### Additional context


Discussion:
ClementPinard: Hi, is this improvement proposition considered ? I have created this PR as a draft to spark some discussion about changing autosummary behaviour, but otherwise it could be reviewed as is.

I really think the current behaviour is not one users would expect when they use the recursive option.
AA-Turner: Please may you add a test of the new behaviour, and an entry to the changelog (`CHANGES`)?

A
ClementPinard: I think I have found a solution that works.

If `__all__` is given , first look for imported modules (possibly renamed) and then look for the rest.

Additionally, there is now a skip list for get_modules, to avoid tryinbg to get documentation from a module that was overwritten in the package namespace

This will cause this kind of code to not crash with sphinx :

(file `package/__init__.py`)

```python
from .a import a
```

autosummary will now know that `package.a` is a function and not a module anymore.

NB: Yes this does happen even though I would personally consider it an antipattern. Some JAVA inspired python code exist out there with only a class in the module and a class the same name of the module and then an import similar to the aforementionned one in the parent package `__init__.py`. Whether we should try to be compliant with this or not is debatable but it it's possible to do that in python, so it will not disappear.

On a more genral note, I have seen that the difference between private members and public members is the leading underscore (see https://github.com/sphinx-doc/sphinx/blob/master/sphinx/ext/autosummary/generate.py#L283).

Without a proper `__all__` list it makes sense, but if it is given (and the `autosummary_ignore_module_all` is False), I think it should prevail as it is by definition the public API. It's out of the scope of this PR, but I can add this modification as well if you want.

Patch:
diff --git a/CHANGES b/CHANGES
index fe7a0a8c4..0097c68e7 100644
--- a/CHANGES
+++ b/CHANGES
@@ -27,6 +27,9 @@ Deprecated
 Features added
 --------------
 
+* #10811: Autosummary: extend ``__all__`` to imported members for template rendering
+  when option ``autosummary_ignore_module_all`` is set to ``False``. Patch by
+  Clement Pinard
 * #11147: Add a ``content_offset`` parameter to ``nested_parse_with_titles()``,
   allowing for correct line numbers during nested parsing.
   Patch by Jeremy Maitin-Shepard
diff --git a/sphinx/ext/autosummary/generate.py b/sphinx/ext/autosummary/generate.py
index 9b9abdf26..b74ce0142 100644
--- a/sphinx/ext/autosummary/generate.py
+++ b/sphinx/ext/autosummary/generate.py
@@ -300,9 +300,17 @@ def get_module_attrs(members: Any) -> tuple[list[str], list[str]]:
             pass    # give up if ModuleAnalyzer fails to parse code
         return public, attrs
 
-    def get_modules(obj: Any) -> tuple[list[str], list[str]]:
+    def get_modules(
+            obj: Any,
+            skip: Sequence[str],
+            public_members: Sequence[str] | None = None) -> tuple[list[str], list[str]]:
         items: list[str] = []
+        public: list[str] = []
         for _, modname, _ispkg in pkgutil.iter_modules(obj.__path__):
+
+            if modname in skip:
+                # module was overwritten in __init__.py, so not accessible
+                continue
             fullname = name + '.' + modname
             try:
                 module = import_module(fullname)
@@ -312,7 +320,12 @@ def get_modules(obj: Any) -> tuple[list[str], list[str]]:
                 pass
 
             items.append(fullname)
-        public = [x for x in items if not x.split('.')[-1].startswith('_')]
+            if public_members is not None:
+                if modname in public_members:
+                    public.append(fullname)
+            else:
+                if not modname.startswith('_'):
+                    public.append(fullname)
         return public, items
 
     ns: dict[str, Any] = {}
@@ -321,6 +334,10 @@ def get_modules(obj: Any) -> tuple[list[str], list[str]]:
     if doc.objtype == 'module':
         scanner = ModuleScanner(app, obj)
         ns['members'] = scanner.scan(imported_members)
+
+        respect_module_all = not app.config.autosummary_ignore_module_all
+        imported_members = imported_members or ('__all__' in dir(obj) and respect_module_all)
+
         ns['functions'], ns['all_functions'] = \
             get_members(obj, {'function'}, imported=imported_members)
         ns['classes'], ns['all_classes'] = \
@@ -331,7 +348,36 @@ def get_modules(obj: Any) -> tuple[list[str], list[str]]:
             get_module_attrs(ns['members'])
         ispackage = hasattr(obj, '__path__')
         if ispackage and recursive:
-            ns['modules'], ns['all_modules'] = get_modules(obj)
+            # Use members that are not modules as skip list, because it would then mean
+            # that module was overwritten in the package namespace
+            skip = (
+                ns["all_functions"]
+                + ns["all_classes"]
+                + ns["all_exceptions"]
+                + ns["all_attributes"]
+            )
+
+            # If respect_module_all and module has a __all__ attribute, first get
+            # modules that were explicitly imported. Next, find the rest with the
+            # get_modules method, but only put in "public" modules that are in the
+            # __all__ list
+            #
+            # Otherwise, use get_modules method normally
+            if respect_module_all and '__all__' in dir(obj):
+                imported_modules, all_imported_modules = \
+                    get_members(obj, {'module'}, imported=True)
+                skip += all_imported_modules
+                imported_modules = [name + '.' + modname for modname in imported_modules]
+                all_imported_modules = \
+                    [name + '.' + modname for modname in all_imported_modules]
+                public_members = getall(obj)
+            else:
+                imported_modules, all_imported_modules = [], []
+                public_members = None
+
+            modules, all_modules = get_modules(obj, skip=skip, public_members=public_members)
+            ns['modules'] = imported_modules + modules
+            ns["all_modules"] = all_imported_modules + all_modules
     elif doc.objtype == 'class':
         ns['members'] = dir(obj)
         ns['inherited_members'] = \


Test patch:
diff --git a/tests/roots/test-ext-autosummary-imported_members/index.rst b/tests/roots/test-ext-autosummary-imported_members/index.rst
index 608ca2954..1c551265e 100644
--- a/tests/roots/test-ext-autosummary-imported_members/index.rst
+++ b/tests/roots/test-ext-autosummary-imported_members/index.rst
@@ -1,5 +1,5 @@
-test-ext-autosummary-mock_imports
-=================================
+test-ext-autosummary-imported_members
+=====================================
 
 .. autosummary::
    :toctree: generated
diff --git a/tests/roots/test-ext-autosummary-module_all/autosummary_dummy_package_all/__init__.py b/tests/roots/test-ext-autosummary-module_all/autosummary_dummy_package_all/__init__.py
new file mode 100644
index 000000000..82f2060fb
--- /dev/null
+++ b/tests/roots/test-ext-autosummary-module_all/autosummary_dummy_package_all/__init__.py
@@ -0,0 +1,13 @@
+from .autosummary_dummy_module import Bar, PublicBar, foo, public_foo
+
+
+def baz():
+    """Baz function"""
+    pass
+
+
+def public_baz():
+    """Public Baz function"""
+
+
+__all__ = ["PublicBar", "public_foo", "public_baz", "extra_dummy_module"]
diff --git a/tests/roots/test-ext-autosummary-module_all/autosummary_dummy_package_all/autosummary_dummy_module.py b/tests/roots/test-ext-autosummary-module_all/autosummary_dummy_package_all/autosummary_dummy_module.py
new file mode 100644
index 000000000..ef89e22f4
--- /dev/null
+++ b/tests/roots/test-ext-autosummary-module_all/autosummary_dummy_package_all/autosummary_dummy_module.py
@@ -0,0 +1,20 @@
+class Bar:
+    """Bar class"""
+
+    pass
+
+
+class PublicBar:
+    """Public Bar class"""
+
+    pass
+
+
+def foo():
+    """Foo function"""
+    pass
+
+
+def public_foo():
+    """Public Foo function"""
+    pass
diff --git a/tests/roots/test-ext-autosummary-module_all/autosummary_dummy_package_all/extra_dummy_module.py b/tests/roots/test-ext-autosummary-module_all/autosummary_dummy_package_all/extra_dummy_module.py
new file mode 100644
index 000000000..ef89e22f4
--- /dev/null
+++ b/tests/roots/test-ext-autosummary-module_all/autosummary_dummy_package_all/extra_dummy_module.py
@@ -0,0 +1,20 @@
+class Bar:
+    """Bar class"""
+
+    pass
+
+
+class PublicBar:
+    """Public Bar class"""
+
+    pass
+
+
+def foo():
+    """Foo function"""
+    pass
+
+
+def public_foo():
+    """Public Foo function"""
+    pass
diff --git a/tests/roots/test-ext-autosummary-module_all/conf.py b/tests/roots/test-ext-autosummary-module_all/conf.py
new file mode 100644
index 000000000..c6ff53419
--- /dev/null
+++ b/tests/roots/test-ext-autosummary-module_all/conf.py
@@ -0,0 +1,8 @@
+import os
+import sys
+
+sys.path.insert(0, os.path.abspath('.'))
+
+extensions = ['sphinx.ext.autosummary']
+autosummary_generate = True
+autosummary_ignore_module_all = False
diff --git a/tests/roots/test-ext-autosummary-module_all/index.rst b/tests/roots/test-ext-autosummary-module_all/index.rst
new file mode 100644
index 000000000..cd638ad35
--- /dev/null
+++ b/tests/roots/test-ext-autosummary-module_all/index.rst
@@ -0,0 +1,8 @@
+test-ext-autosummary-module_all
+===============================
+
+.. autosummary::
+   :toctree: generated
+   :recursive:
+
+   autosummary_dummy_package_all
diff --git a/tests/test_ext_autosummary.py b/tests/test_ext_autosummary.py
index f8c2de495..69b4b76bc 100644
--- a/tests/test_ext_autosummary.py
+++ b/tests/test_ext_autosummary.py
@@ -545,6 +545,30 @@ def test_autosummary_imported_members(app, status, warning):
         sys.modules.pop('autosummary_dummy_package', None)
 
 
+@pytest.mark.sphinx('dummy', testroot='ext-autosummary-module_all')
+def test_autosummary_module_all(app, status, warning):
+    try:
+        app.build()
+        # generated/foo is generated successfully
+        assert app.env.get_doctree('generated/autosummary_dummy_package_all')
+        module = (app.srcdir / 'generated' / 'autosummary_dummy_package_all.rst').read_text(encoding='utf8')
+        assert ('   .. autosummary::\n'
+                '   \n'
+                '      PublicBar\n'
+                '   \n' in module)
+        assert ('   .. autosummary::\n'
+                '   \n'
+                '      public_foo\n'
+                '      public_baz\n'
+                '   \n' in module)
+        assert ('.. autosummary::\n'
+                '   :toctree:\n'
+                '   :recursive:\n\n'
+                '   autosummary_dummy_package_all.extra_dummy_module\n\n' in module)
+    finally:
+        sys.modules.pop('autosummary_dummy_package_all', None)
+
+
 @pytest.mark.sphinx(testroot='ext-autodoc',
                     confoverrides={'extensions': ['sphinx.ext.autosummary']})
 def test_generate_autosummary_docs_property(app):