From c59381496a3743d4ee2221505d84eff610ce7d2d Mon Sep 17 00:00:00 2001 From: =?utf8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Fri, 12 Jul 2019 01:03:42 +0200 Subject: [PATCH] documentation/python: properly handle recursively imported modules. Basically converting the crawl to be breadth-first instead of depth-first and maintaining a set of crawled modules to ensure they're never visited again. This could happen with classes as well, but it's rarer -- one would need to add it to __all__ twice (or once and then it gets discovered implicitly) etc. For now I'm simply asserting, will revisit in case it proves to be an actual problem. --- documentation/python.py | 56 ++++++++++++-- .../inspect_recursive.a.html | 31 ++++++++ .../inspect_recursive.first.html | 31 ++++++++ .../inspect_recursive/inspect_recursive.html | 73 +++++++++++++++++++ .../inspect_recursive/__init__.py | 18 +++++ .../inspect_recursive/first.py | 5 ++ .../inspect_recursive/second.py | 6 ++ documentation/test_python/test_inspect.py | 10 +++ 8 files changed, 224 insertions(+), 6 deletions(-) create mode 100644 documentation/test_python/inspect_recursive/inspect_recursive.a.html create mode 100644 documentation/test_python/inspect_recursive/inspect_recursive.first.html create mode 100644 documentation/test_python/inspect_recursive/inspect_recursive.html create mode 100644 documentation/test_python/inspect_recursive/inspect_recursive/__init__.py create mode 100644 documentation/test_python/inspect_recursive/inspect_recursive/first.py create mode 100644 documentation/test_python/inspect_recursive/inspect_recursive/second.py diff --git a/documentation/python.py b/documentation/python.py index 996e2691..2b79f402 100755 --- a/documentation/python.py +++ b/documentation/python.py @@ -161,6 +161,8 @@ class State: self.name_map: Dict[str, Empty] = {} + self.crawled: Set[object] = set() + def map_name_prefix(state: State, type: str) -> str: for prefix, replace in state.module_mapping.items(): if type == prefix or type.startswith(prefix + '.'): @@ -240,6 +242,14 @@ _filtered_builtin_properties = set([ ]) def crawl_class(state: State, path: List[str], class_): + assert inspect.isclass(class_) + + # TODO: if this fires, it means there's a class duplicated in more than one + # __all__ (or it gets picked up implicitly and then in __all__) -- how to + # handle gracefully? + assert id(class_) not in state.crawled + state.crawled.add(id(class_)) + class_entry = Empty() class_entry.type = EntryType.CLASS class_entry.object = class_ @@ -288,13 +298,29 @@ def crawl_class(state: State, path: List[str], class_): # Add itself to the name map state.name_map['.'.join(path)] = class_entry -def crawl_module(state: State, path: List[str], module): +def crawl_module(state: State, path: List[str], module) -> List[Tuple[List[str], object]]: + assert inspect.ismodule(module) + + # Assume this module is not crawled yet -- the parent crawl shouldn't even + # put it to members if it's crawled already. Otherwise add itself to + # the list of crawled objects to avoid going through it again. + assert id(module) not in state.crawled + state.crawled.add(id(module)) + + # This module isn't a duplicate, thus we can now safely add itself to + # parent's members (if there's a parent) + if len(path) > 1: state.name_map['.'.join(path[:-1])].members += [path[-1]] + module_entry = Empty() module_entry.type = EntryType.MODULE module_entry.object = module module_entry.path = path module_entry.members = [] + # This gets returned to ensure the modules get processed in a breadth-first + # order + submodules_to_crawl: List[Tuple[List[str], object]] = [] + # This is actually complicated -- if the module defines __all__, use that. # The __all__ is meant to expose the public API, so we don't filter out # underscored things. @@ -340,7 +366,14 @@ def crawl_module(state: State, path: List[str], module): logging.warning("unknown symbol %s in %s", name, '.'.join(path)) continue elif type == EntryType.MODULE: - crawl_module(state, subpath, object) + # TODO: this might fire if a module is in __all__ after it was + # picked up implicitly before -- how to handle gracefully? + assert id(object) not in state.crawled + submodules_to_crawl += [(subpath, object)] + # Not adding to module_entry.members, done by the nested + # crawl_module() itself if it is sure that it isn't a + # duplicated module + continue elif type == EntryType.CLASS: crawl_class(state, subpath, object) else: @@ -405,7 +438,11 @@ def crawl_module(state: State, path: List[str], module): # Ignore unknown object types (with __all__ we warn instead) continue elif type == EntryType.MODULE: - crawl_module(state, subpath, object) + submodules_to_crawl += [(subpath, object)] + # Not adding to module_entry.members, done by the nested + # crawl_module() itself if it is sure that it isn't a + # duplicated module + continue elif type == EntryType.CLASS: crawl_class(state, subpath, object) else: @@ -421,6 +458,8 @@ def crawl_module(state: State, path: List[str], module): # Add itself to the name map state.name_map['.'.join(path)] = module_entry + return submodules_to_crawl + _pybind_name_rx = re.compile('[a-zA-Z0-9_]*') _pybind_arg_name_rx = re.compile('[*a-zA-Z0-9_]+') _pybind_type_rx = re.compile('[a-zA-Z0-9_.]+') @@ -1187,17 +1226,22 @@ def run(basedir, config, templates): for hook in state.hooks_pre_page: hook() # Crawl all input modules to gather the name tree, put their names into a - # list for the index + # list for the index. The crawl is done breadth-first, so the function + # returns a list of submodules to be crawled next. class_index = [] + modules_to_crawl = [] for module in config['INPUT_MODULES']: if isinstance(module, str): module_name = module module = importlib.import_module(module) else: module_name = module.__name__ - - crawl_module(state, [module_name], module) + modules_to_crawl += [([module_name], module)] class_index += [module_name] + while modules_to_crawl: + path, object = modules_to_crawl.pop(0) + if id(object) in state.crawled: continue + modules_to_crawl += crawl_module(state, path, object) # Add special pages to the name map. The pages are done after so they can # override these. diff --git a/documentation/test_python/inspect_recursive/inspect_recursive.a.html b/documentation/test_python/inspect_recursive/inspect_recursive.a.html new file mode 100644 index 00000000..69333f45 --- /dev/null +++ b/documentation/test_python/inspect_recursive/inspect_recursive.a.html @@ -0,0 +1,31 @@ + + + + + inspect_recursive.a | My Python Project + + + + + +
+
+
+
+
+

+ inspect_recursive.a module +

+

Second module, imported as inspect_recursive.a, with no contents

+
+
+
+
+ + diff --git a/documentation/test_python/inspect_recursive/inspect_recursive.first.html b/documentation/test_python/inspect_recursive/inspect_recursive.first.html new file mode 100644 index 00000000..c53e9266 --- /dev/null +++ b/documentation/test_python/inspect_recursive/inspect_recursive.first.html @@ -0,0 +1,31 @@ + + + + + inspect_recursive.first | My Python Project + + + + + +
+
+
+
+
+

+ inspect_recursive.first module +

+

First module, imported as inspect_recursive.first, with no contents

+
+
+
+
+ + diff --git a/documentation/test_python/inspect_recursive/inspect_recursive.html b/documentation/test_python/inspect_recursive/inspect_recursive.html new file mode 100644 index 00000000..a8668b61 --- /dev/null +++ b/documentation/test_python/inspect_recursive/inspect_recursive.html @@ -0,0 +1,73 @@ + + + + + inspect_recursive | My Python Project + + + + + +
+
+
+
+
+

+ inspect_recursive module +

+

Recursive imports

+
+

Contents

+ +
+
+

Modules

+
+
module a
+
Second module, imported as inspect_recursive.a, with no contents
+
module first
+
First module, imported as inspect_recursive.first, with no contents
+
+
+
+

Classes

+
+
class Foo
+
A class
+
+
+
+

Functions

+
+
+ def bar() -> Foo +
+
Function that also returns Foo
+
+ def foo() -> Foo +
+
Function that returns Foo
+
+
+
+
+
+
+ + diff --git a/documentation/test_python/inspect_recursive/inspect_recursive/__init__.py b/documentation/test_python/inspect_recursive/inspect_recursive/__init__.py new file mode 100644 index 00000000..a512bbb2 --- /dev/null +++ b/documentation/test_python/inspect_recursive/inspect_recursive/__init__.py @@ -0,0 +1,18 @@ +"""Recursive imports""" + +import inspect_recursive # Importing self, should get ignored + +class Foo: + """A class""" + +from . import first + +# Importing a module twice, only one of them should be picked +from . import second as b +from . import second as a + +def foo() -> b.Bar: + """Function that returns Foo""" + +def bar() -> a.Bar: + """Function that also returns Foo""" diff --git a/documentation/test_python/inspect_recursive/inspect_recursive/first.py b/documentation/test_python/inspect_recursive/inspect_recursive/first.py new file mode 100644 index 00000000..15ad4591 --- /dev/null +++ b/documentation/test_python/inspect_recursive/inspect_recursive/first.py @@ -0,0 +1,5 @@ +"""First module, imported as inspect_recursive.first, with no contents""" + +import inspect_recursive + +from inspect_recursive import Foo as Bar diff --git a/documentation/test_python/inspect_recursive/inspect_recursive/second.py b/documentation/test_python/inspect_recursive/inspect_recursive/second.py new file mode 100644 index 00000000..2b96dc19 --- /dev/null +++ b/documentation/test_python/inspect_recursive/inspect_recursive/second.py @@ -0,0 +1,6 @@ +"""Second module, imported as inspect_recursive.a, with no contents""" + +import inspect_recursive.first as a +import inspect_recursive.second as b + +from inspect_recursive import Foo as Bar diff --git a/documentation/test_python/test_inspect.py b/documentation/test_python/test_inspect.py index 6920b9ed..e7ee4612 100644 --- a/documentation/test_python/test_inspect.py +++ b/documentation/test_python/test_inspect.py @@ -136,3 +136,13 @@ class NameMapping(BaseInspectTestCase): self.assertEqual(*self.actual_expected_contents('inspect_name_mapping.html')) self.assertEqual(*self.actual_expected_contents('inspect_name_mapping.Class.html')) self.assertEqual(*self.actual_expected_contents('inspect_name_mapping.submodule.html')) + +class Recursive(BaseInspectTestCase): + def __init__(self, *args, **kwargs): + super().__init__(__file__, 'recursive', *args, **kwargs) + + def test(self): + self.run_python() + self.assertEqual(*self.actual_expected_contents('inspect_recursive.html')) + self.assertEqual(*self.actual_expected_contents('inspect_recursive.first.html')) + self.assertEqual(*self.actual_expected_contents('inspect_recursive.a.html')) -- 2.30.2