[autosummary] remove recursion limit and module/package separation

19 files changed, 63 insertions, 123 deletions

diff --git a/doc/usage/extensions/autosummary.rst b/doc/usage/extensions/autosummary.rst
index 0bdc4e640..316ffcb9d 100644
--- a/doc/usage/extensions/autosummary.rst
+++ b/doc/usage/extensions/autosummary.rst
@@ -25,7 +25,7 @@ The :mod:`sphinx.ext.autosummary` extension does this in three parts:
    These files by default contain only the corresponding
    :mod:`sphinx.ext.autodoc` directive, but can be customized with templates.
 
-3. Optionally, :confval:`autosummary_depth_limit` config value can be
+3. Optionally, the :confval:`autosummary_recursive` config value can be
    used to generate "stub" files of modules and sub-packages for packages
    under the :rst:dir:`autosummary` directive.
 
@@ -148,16 +148,12 @@ also use these config values:
    The new files will be placed in the directories specified in the
    ``:toctree:`` options of the directives.
 
-.. confval:: autosummary_depth_limit
+.. confval:: autosummary_recursive
 
-   Integer that determines the maximal depth (starting from root) when adding
-   modules and sub-packages of packages.
-   
-    - ``autosummary_depth_limit = -1``: no limit
-    - ``autosummary_depth_limit = 0``: disable adding sub-packages (default)
-    - ``autosummary_depth_limit > 0``: limited depth starting from root
+   Boolean that determines whether to add modules and sub-packages
+   recursively. It is disabled by default.
 
-   .. versionadded:: 2.1
+   .. versionadded:: 2.2
 
 .. confval:: autosummary_mock_imports
 
@@ -272,14 +268,7 @@ The following variables available in the templates:
    List containing names of "public" modules in the package.  Only available for
    modules that are packages.
 
-   .. versionadded:: 2.1
-
-.. data:: packages
-
-   List containing names of "public" sub-packages in the package.  Only available
-   for modules that are packages.
-
-   .. versionadded:: 2.1
+   .. versionadded:: 2.2
 
 Additionally, the following filters are available
 
diff --git a/sphinx/ext/autosummary/__init__.py b/sphinx/ext/autosummary/__init__.py
index cf768dc4f..87ae90386 100644
--- a/sphinx/ext/autosummary/__init__.py
+++ b/sphinx/ext/autosummary/__init__.py
@@ -751,14 +751,14 @@ def process_generate_options(app):
                           'But your source_suffix does not contain .rst. Skipped.'))
         return
 
-    depth_limit = app.config.autosummary_depth_limit
     imported_members = app.config.autosummary_imported_members
+    recursive = app.config.autosummary_recursive
     with mock(app.config.autosummary_mock_imports):
         generate_autosummary_docs(genfiles, builder=app.builder,
                                   warn=logger.warning, info=logger.info,
                                   suffix=suffix, base_path=app.srcdir,
                                   app=app, imported_members=imported_members,
-                                  depth_limit=depth_limit)
+                                  recursive=recursive)
 
 
 def setup(app):
@@ -782,7 +782,7 @@ def setup(app):
     app.connect('doctree-read', process_autosummary_toc)
     app.connect('builder-inited', process_generate_options)
     app.add_config_value('autosummary_generate', [], True, [bool])
-    app.add_config_value('autosummary_depth_limit', 0, 'env', [int])
+    app.add_config_value('autosummary_recursive', False, 'env', [bool])
     app.add_config_value('autosummary_mock_imports',
                          lambda config: config.autodoc_mock_imports, 'env')
     app.add_config_value('autosummary_imported_members', [], False, [bool])
diff --git a/sphinx/ext/autosummary/generate.py b/sphinx/ext/autosummary/generate.py
index 129c43f65..5fcb1fe96 100644
--- a/sphinx/ext/autosummary/generate.py
+++ b/sphinx/ext/autosummary/generate.py
@@ -136,8 +136,8 @@ def generate_autosummary_docs(sources,                 # type: List[str]
                               builder=None,            # type: Builder
                               template_dir=None,       # type: str
                               imported_members=False,  # type: bool
+                              recursive=False,         # type: bool
                               app=None,                # type: Any
-                              depth_limit=0,           # type: int
                               ):
     # type: (...) -> None
     showed_sources = list(sorted(sources))
@@ -180,22 +180,16 @@ def generate_autosummary_docs(sources,                 # type: List[str]
         # ... any module that contains a __path__ attribute is
         # considered a package ...
         ispackage = hasattr(obj, '__path__')
-        if ispackage:
-            depth = len(name.split('.')) - 1
-            add_package_children = depth < depth_limit or depth_limit < 0
-            if add_package_children:
-                info(__('[autosummary] add modules/packages from %s') % repr(name))
-        else:
-            add_package_children = False
+        if ispackage and recursive:
+            info(__('[autosummary] add modules/packages from %s') % repr(name))
 
         fn = os.path.join(path, name + suffix)
 
         # Skip it if it exists and not a package.
         if os.path.isfile(fn):
-            if ispackage and depth_limit != 0:
-                # Overwrite the file because submodules/packages
-                # could have been added/removed from the package
-                # or the depth limit might have changed.
+            if ispackage and recursive:
+                # Overwrite the file because modules/subpackages
+                # could have been added/removed from the package.
                 # Warning: this file could have been created by the user
                 # in which case it is lost.
                 warn('[autosummary] overwriting docs of package %s' % (repr(name)))
@@ -230,10 +224,9 @@ def generate_autosummary_docs(sources,                 # type: List[str]
                           if x in include_public or not x.startswith('_')]
                 return public, items
 
-            def get_package_members(obj, typ, include_public=[]):
+            def get_modules(obj, include_public=[]):
                 # type: (Any, str, List[str]) -> Tuple[List[str], List[str]]
                 items = []  # type: List[str]
-                pkg_required = typ == 'package'
                 for _, modname, ispkg in pkgutil.iter_modules(obj.__path__):
                     fullname = name + '.' + modname
                     try:
@@ -241,8 +234,7 @@ def generate_autosummary_docs(sources,                 # type: List[str]
                     except ImportError as e:
                         warn('[autosummary] failed to import %s: %s' % (fullname, e))
                         continue
-                    if ispkg == pkg_required:
-                        items.append(fullname)
+                    items.append(fullname)
                 public = [x for x in items
                           if x in include_public or not x.split('.')[-1].startswith('_')]
                 return public, items
@@ -257,11 +249,9 @@ def generate_autosummary_docs(sources,                 # type: List[str]
                     get_members(obj, {'class'}, imported=imported_members)
                 ns['exceptions'], ns['all_exceptions'] = \
                     get_members(obj,  {'exception'}, imported=imported_members)
-                if add_package_children:
+                if ispackage and recursive:
                     ns['modules'], ns['all_modules'] = \
-                        get_package_members(obj, 'module')
-                    ns['packages'], ns['all_packages'] = \
-                        get_package_members(obj, 'package')
+                        get_modules(obj)
             elif doc.objtype == 'class':
                 ns['members'] = dir(obj)
                 ns['inherited_members'] = \
@@ -298,7 +288,7 @@ def generate_autosummary_docs(sources,                 # type: List[str]
                                   base_path=base_path, builder=builder,
                                   template_dir=template_dir,
                                   imported_members=imported_members,
-                                  depth_limit=depth_limit,
+                                  recursive=recursive,
                                   app=app)
 
 
@@ -480,7 +470,7 @@ def main(argv=sys.argv[1:]):
                               '.' + args.suffix,
                               template_dir=args.templates,
                               imported_members=args.imported_members,
-                              depth_limit=args.depth_limit,
+                              recursive=args.recursive,
                               app=app)
 
 
diff --git a/sphinx/ext/autosummary/templates/autosummary/module.rst b/sphinx/ext/autosummary/templates/autosummary/module.rst
index 049498021..b9644ebca 100644
--- a/sphinx/ext/autosummary/templates/autosummary/module.rst
+++ b/sphinx/ext/autosummary/templates/autosummary/module.rst
@@ -46,15 +46,3 @@
 {%- endfor %}
 {% endif %}
 {% endblock %}
-
-{% block packages %}
-{% if packages %}
-.. rubric:: packages
-
-.. autosummary::
-   :toctree: packages
-{% for item in packages %}
-   {{ item }}
-{%- endfor %}
-{% endif %}
-{% endblock %}
diff --git a/tests/roots/test-ext-autosummary-package/conf.py b/tests/roots/test-ext-autosummary-recursive/conf.py
index 1c0d02202..1c0d02202 100644
--- a/tests/roots/test-ext-autosummary-package/conf.py
+++ b/tests/roots/test-ext-autosummary-recursive/conf.py
diff --git a/tests/roots/test-ext-autosummary-package/index.rst b/tests/roots/test-ext-autosummary-recursive/index.rst
index d5292607c..d5292607c 100644
--- a/tests/roots/test-ext-autosummary-package/index.rst
+++ b/tests/roots/test-ext-autosummary-recursive/index.rst
diff --git a/tests/roots/test-ext-autosummary-package/package/__init__.py b/tests/roots/test-ext-autosummary-recursive/package/__init__.py
index e69de29bb..e69de29bb 100644
--- a/tests/roots/test-ext-autosummary-package/package/__init__.py
+++ b/tests/roots/test-ext-autosummary-recursive/package/__init__.py
diff --git a/tests/roots/test-ext-autosummary-package/package/module.py b/tests/roots/test-ext-autosummary-recursive/package/module.py
index 5506d0bc9..5506d0bc9 100644
--- a/tests/roots/test-ext-autosummary-package/package/module.py
+++ b/tests/roots/test-ext-autosummary-recursive/package/module.py
diff --git a/tests/roots/test-ext-autosummary-package/package/module_importfail.py b/tests/roots/test-ext-autosummary-recursive/package/module_importfail.py
index 9e3f9f195..9e3f9f195 100644
--- a/tests/roots/test-ext-autosummary-package/package/module_importfail.py
+++ b/tests/roots/test-ext-autosummary-recursive/package/module_importfail.py
diff --git a/tests/roots/test-ext-autosummary-package/package/package/__init__.py b/tests/roots/test-ext-autosummary-recursive/package/package/__init__.py
index e69de29bb..e69de29bb 100644
--- a/tests/roots/test-ext-autosummary-package/package/package/__init__.py
+++ b/tests/roots/test-ext-autosummary-recursive/package/package/__init__.py
diff --git a/tests/roots/test-ext-autosummary-package/package/package/module.py b/tests/roots/test-ext-autosummary-recursive/package/package/module.py
index 5506d0bc9..5506d0bc9 100644
--- a/tests/roots/test-ext-autosummary-package/package/package/module.py
+++ b/tests/roots/test-ext-autosummary-recursive/package/package/module.py
diff --git a/tests/roots/test-ext-autosummary-package/package/package/module_importfail.py b/tests/roots/test-ext-autosummary-recursive/package/package/module_importfail.py
index 9e3f9f195..9e3f9f195 100644
--- a/tests/roots/test-ext-autosummary-package/package/package/module_importfail.py
+++ b/tests/roots/test-ext-autosummary-recursive/package/package/module_importfail.py
diff --git a/tests/roots/test-ext-autosummary-package/package/package/package/__init__.py b/tests/roots/test-ext-autosummary-recursive/package/package/package/__init__.py
index e69de29bb..e69de29bb 100644
--- a/tests/roots/test-ext-autosummary-package/package/package/package/__init__.py
+++ b/tests/roots/test-ext-autosummary-recursive/package/package/package/__init__.py
diff --git a/tests/roots/test-ext-autosummary-package/package/package/package/module.py b/tests/roots/test-ext-autosummary-recursive/package/package/package/module.py
index 5506d0bc9..5506d0bc9 100644
--- a/tests/roots/test-ext-autosummary-package/package/package/package/module.py
+++ b/tests/roots/test-ext-autosummary-recursive/package/package/package/module.py
diff --git a/tests/roots/test-ext-autosummary-package/package/package/package/module_importfail.py b/tests/roots/test-ext-autosummary-recursive/package/package/package/module_importfail.py
index 9e3f9f195..9e3f9f195 100644
--- a/tests/roots/test-ext-autosummary-package/package/package/package/module_importfail.py
+++ b/tests/roots/test-ext-autosummary-recursive/package/package/package/module_importfail.py
diff --git a/tests/roots/test-ext-autosummary-package/package/package/package/package/__init__.py b/tests/roots/test-ext-autosummary-recursive/package/package/package/package/__init__.py
index e69de29bb..e69de29bb 100644
--- a/tests/roots/test-ext-autosummary-package/package/package/package/package/__init__.py
+++ b/tests/roots/test-ext-autosummary-recursive/package/package/package/package/__init__.py
diff --git a/tests/roots/test-ext-autosummary-package/package/package/package/package/module.py b/tests/roots/test-ext-autosummary-recursive/package/package/package/package/module.py
index 5506d0bc9..5506d0bc9 100644
--- a/tests/roots/test-ext-autosummary-package/package/package/package/package/module.py
+++ b/tests/roots/test-ext-autosummary-recursive/package/package/package/package/module.py
diff --git a/tests/roots/test-ext-autosummary-package/package/package/package/package/module_importfail.py b/tests/roots/test-ext-autosummary-recursive/package/package/package/package/module_importfail.py
index 9e3f9f195..9e3f9f195 100644
--- a/tests/roots/test-ext-autosummary-package/package/package/package/package/module_importfail.py
+++ b/tests/roots/test-ext-autosummary-recursive/package/package/package/package/module_importfail.py
diff --git a/tests/test_ext_autosummary.py b/tests/test_ext_autosummary.py
index c58fdc0e1..bdfea0633 100644
--- a/tests/test_ext_autosummary.py
+++ b/tests/test_ext_autosummary.py
@@ -224,16 +224,9 @@ def test_autosummary_generate(app, status, warning):
             '   \n' in Foo)
 
 
-def _assert_autosummary_generate_package(app):
+def _assert_autosummary_recursive(app):
     app.builder.build_all()
 
-    # Prepare package exploration
-    max_depth = 3
-    depth_limit = app.config.autosummary_depth_limit
-    assert depth_limit <= max_depth
-    
-    unlimited = depth_limit < 0
-
     # All packages, modules and classes have the same name
     package_name = 'package'
     module_name = 'module'
@@ -242,18 +235,21 @@ def _assert_autosummary_generate_package(app):
     nsuffix = len(extension)+1
 
     # Expected module.rst template formatting
-    package_rubic_name = 'packages'
+    recursive = app.config.autosummary_recursive
     module_rubic_name = 'modules'
-    package_rubic = '.. rubric:: ' + package_rubic_name
     module_rubic = '.. rubric:: ' + module_rubic_name
-    package_summary = '.. autosummary::\n'\
-                      '   :toctree: {}\n'\
-                      '\n'\
-                      '   {{}}.{}\n'.format(package_rubic_name, package_name)
     module_summary = '.. autosummary::\n'\
                      '   :toctree: {}\n'\
                      '\n'\
-                     '   {{}}.{}\n'.format(module_rubic_name, module_name)
+                     '   {{}}.{}\n'\
+                     '   {{}}.{}\n'.format(module_rubic_name,
+                                           module_name,
+                                           package_name)
+    last_module_summary = '.. autosummary::\n'\
+                          '   :toctree: {}\n'\
+                          '\n'\
+                          '   {{}}.{}\n'.format(module_rubic_name,
+                                                module_name)
     class_summary = '   .. autosummary::\n'\
                     '   \n'\
                     '      {}\n'\
@@ -264,18 +260,15 @@ def _assert_autosummary_generate_package(app):
         content = file.text()
         expected = ['.. automodule:: ' + pkgroot]
         unexpected = []
-        if has_modules:
+        if recursive:
             lst = expected
         else:
             lst = unexpected
         lst.append(module_rubic)
-        lst.append(module_summary.format(pkgroot))
-        if has_packages:
-            lst = expected
+        if depth == max_depth:
+            lst.append(last_module_summary.format(pkgroot))
         else:
-            lst = unexpected
-        lst.append(package_rubic)
-        lst.append(package_summary.format(pkgroot))
+            lst.append(module_summary.format(pkgroot, pkgroot))
         for text in expected:
             assert text in content
         for text in unexpected:
@@ -289,75 +282,55 @@ def _assert_autosummary_generate_package(app):
         assert class_summary in content
 
     root = app.srcdir / 'generated'
-    depth = 0
     pkgroot = ''
-    packages = []
-    modules = []
-    directories = []
-    while True:
-        limit_reached = (depth == depth_limit) and not unlimited
-        last_package = depth == max_depth
-        has_packages = not limit_reached and not last_package
-        has_modules = not limit_reached
-
+    expected_paths = []
+    if app.config.autosummary_recursive:
+        max_depth = 3
+    else:
+        max_depth = 0
+    for depth in range(max_depth+1):
         if pkgroot:
             pkgroot = '.'.join((pkgroot, package_name))
         else:
             pkgroot = package_name
         package = root / '.'.join((pkgroot, extension))
-        packages.append(package)
+        expected_paths.append(package)
         assert_package_content(package)
-
-        if limit_reached:
-            break
-
-        directories.append(root / module_rubic_name)
-        module = root / module_rubic_name / \
-            '.'.join((pkgroot, module_name, extension))
-        modules.append(module)
-        assert_module_content(module)
-
-        if last_package:
-            # The last package contains a module but no subpackage
-            break
-
-        directories.append(root / package_rubic_name)
-        root /= package_rubic_name
-        depth += 1
+        if recursive:
+            module = root / module_rubic_name / \
+                '.'.join((pkgroot, module_name, extension))
+            expected_paths.append(root / module_rubic_name)
+            expected_paths.append(module)
+            assert_module_content(module)
+        root /= module_rubic_name
 
     # Check generated files and directories
-    generated = []
+    generated_paths = []
     root = app.srcdir / 'generated'
     for dir, subdirs, files in os.walk(str(root)):
         root = root / dir
         for name in files:
-            generated.append(root / name)
+            generated_paths.append(root / name)
         for name in subdirs:
-            generated.append(root / name)
-    expected = directories + packages + modules
-    assert set(generated) == set(expected)
+            generated_paths.append(root / name)
+    assert set(generated_paths) == set(expected_paths)
 
 
-@pytest.mark.sphinx('dummy', testroot='ext-autosummary-package')
-def test_autosummary_generate_package(make_app, app_params):
+@pytest.mark.sphinx('dummy', testroot='ext-autosummary-recursive')
+def test_autosummary_recursive(make_app, app_params):
     import sphinx.ext.autosummary
     import sphinx.ext.autosummary.generate
     logger = sphinx.ext.autosummary.logger
     args, kwargs = app_params
-    confoverrides = {'autosummary_depth_limit': 0}
+    confoverrides = {'autosummary_recursive': False}
     kwargs['confoverrides'] = confoverrides
-    # Try different depth limits
-    # TODO: going from higher to lower limit
-    # currently fails because generated files
-    # are not deleted
-    for limit in 0, 1, 2, 3, -1:
-        logger.info('autosummary_depth_limit = {}'.format(limit))
-        confoverrides['autosummary_depth_limit'] = limit
+    # Remarks: non-recursive after recursive doesn't work
+    # recursively generated files are not deleted
+    for recursive in False, True:
+        logger.info('autosummary_recursive = {}'.format(recursive))
+        confoverrides['autosummary_recursive'] = recursive
         app = make_app(*args, **kwargs)
-        _assert_autosummary_generate_package(app)
-        # Cleanup
-        # root = app.srcdir / 'generated'
-        # root.rmtree()
+        _assert_autosummary_recursive(app)
 
 
 @pytest.mark.sphinx('latex', **default_kw)