From 810fd115851c641aa038a1968659c98dbd8a48f7 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 24 Aug 2019 18:13:15 +0200 Subject: [PATCH] documentation/python: implement context-relative linking. I didn't change much, but the amount of research needed to verify that I'm indeed doing it (mostly) correct was extreme. There's a pathological case inside a named subclass where I'm doing it differently than Python itself, but I don't expect this to be hit very often, so postponing until later. The tests also assert inside the function definitions (and those functions get called) to verify we're indeed doing (mostly) correct stuff. --- doc/documentation/python.rst | 23 ++++-- documentation/python.py | 26 ++++--- .../test_python/inspect_type_links/docs.rst | 56 +++++++++++++++ .../inspect_type_links.first.Foo.Foo.html | 28 ++++++-- .../inspect_type_links.first.Foo.html | 70 ++++++++++++++++--- .../inspect_type_links.first.html | 2 + .../inspect_type_links.first.sub.Foo.html | 2 + .../inspect_type_links/__init__.py | 6 ++ .../inspect_type_links/first/__init__.py | 63 +++++++++++++---- documentation/test_python/test_inspect.py | 1 + plugins/m/sphinx.py | 15 +++- 11 files changed, 243 insertions(+), 49 deletions(-) create mode 100644 documentation/test_python/inspect_type_links/docs.rst diff --git a/doc/documentation/python.rst b/doc/documentation/python.rst index 9c2b413e..11efd304 100644 --- a/doc/documentation/python.rst +++ b/doc/documentation/python.rst @@ -736,11 +736,24 @@ Keyword argument Content added by the plugin *need* to have :py:`object` set to :py:`None` so the script as well as other plugins can correctly distinguish them. -The :py:`hooks_pre_page` and :py:`hooks_post_run` are called before each page -of output gets rendered (for example, resetting an some internal counter for -page-wide unique element IDs) and after the whole run is done (for example to -serialize cached internal state). Currently, those functions get no arguments -passed. +The :py:`hooks_pre_page` is called before each page of output gets rendered. +Can be used for example for resetting some internal counter for page-wide +unique element IDs. It gets passed the following arguments: + +.. class:: m-table + +=================== =========================================================== +Keyword argument Content +=================== =========================================================== +:py:`path` Path of the module/class/page to render. A list of names, + for modules and classes :py:`'.'.join(path)` is equivalent + to the fully qualified name. Useful to provide + context-sensitive linking capabilities. +=================== =========================================================== + +The :py:`hooks_post_run` is called after the whole run is done, useful for +example to serialize cached internal state. Currently, this function get no +arguments passed. Registration function for a plugin that needs to query the :py:`OUTPUT` setting might look like this --- the remaining keyword arguments will collapse into diff --git a/documentation/python.py b/documentation/python.py index dabfc69a..b45f791e 100755 --- a/documentation/python.py +++ b/documentation/python.py @@ -552,8 +552,12 @@ def make_relative_name(state: State, referrer_path: List[str], name): # Check for ambiguity of the shortened path -- for example, with referrer # being `module.sub.Foo`, target `module.Foo`, the path will get shortened # to `Foo`, making it seem like the target is `module.sub.Foo` instead of - # `module.Foo`. To fix that, the shortened path needs to be `sub.Foo` + # `module.Foo`. To fix that, the shortened path needs to be `module.Foo` # instead of `Foo`. + # + # There's many corner cases, see test_inspect.InspectTypeLinks for the full + # description, tests and verification against python's internal name + # resolution rules. def is_ambiguous(shortened_path): # Concatenate the shortened path with a prefix of the referrer path, # going from longest to shortest, until we find a name that exists. If @@ -561,10 +565,6 @@ def make_relative_name(state: State, referrer_path: List[str], name): # for example, linking from `module.sub` to `module.sub.Foo` can be # done just with `Foo` even though `module.Foo` exists as well, as it's # "closer" to the referrer. - # TODO: See test cases in `inspect_type_links.first.Foo` for very - # *very* pathological cases where we're referencing `Foo` from - # `module.Foo` and there's also `module.Foo.Foo`. Not sure which way is - # better. for i in reversed(range(len(referrer_path))): potentially_ambiguous = referrer_path[:i] + shortened_path if '.'.join(potentially_ambiguous) in state.name_map: @@ -1421,7 +1421,8 @@ def render_module(state: State, path, module, env): logging.debug("generating %s", filename) # Call all registered page begin hooks - for hook in state.hooks_pre_page: hook() + for hook in state.hooks_pre_page: + hook(path=path) page = Empty() page.summary, page.content = extract_docs(state, state.module_docs, path, module.__doc__) @@ -1496,7 +1497,8 @@ def render_class(state: State, path, class_, env): logging.debug("generating %s", filename) # Call all registered page begin hooks - for hook in state.hooks_pre_page: hook() + for hook in state.hooks_pre_page: + hook(path=path) page = Empty() page.summary, page.content = extract_docs(state, state.class_docs, path, class_.__doc__) @@ -1716,7 +1718,8 @@ def render_page(state: State, path, input_filename, env): logging.debug("generating %s", filename) # Call all registered page begin hooks - for hook in state.hooks_pre_page: hook() + for hook in state.hooks_pre_page: + hook(path=path) # Render the file with open(input_filename, 'r') as f: pub = publish_rst(state, f.read(), source_path=input_filename) @@ -1908,9 +1911,6 @@ def run(basedir, config, *, templates=default_templates, search_add_lookahead_ba hooks_pre_page=state.hooks_pre_page, hooks_post_run=state.hooks_post_run) - # Call all registered page begin hooks for the first time - 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. The crawl is done breadth-first, so the function # returns a list of submodules to be crawled next. @@ -1958,6 +1958,10 @@ def run(basedir, config, *, templates=default_templates, search_add_lookahead_ba for hook in state.hooks_post_crawl: hook(name_map=state.name_map) + # Call all registered page begin hooks for the doc rendering + for hook in state.hooks_pre_page: + hook(path=[]) + # Then process the doc input files so we have all data for rendering # module pages. This needs to be done *after* the initial crawl so # cross-linking works as expected. diff --git a/documentation/test_python/inspect_type_links/docs.rst b/documentation/test_python/inspect_type_links/docs.rst new file mode 100644 index 00000000..90ce0219 --- /dev/null +++ b/documentation/test_python/inspect_type_links/docs.rst @@ -0,0 +1,56 @@ +.. py:module:: inspect_type_links + + :ref:`first.Foo` and :ref:`inspect_type_links.first.Foo` should lead to the + same class. + +.. py:module:: inspect_type_links.first + + :ref:`Foo`, :ref:`first.Foo` and :ref:`inspect_type_links.first.Foo` should + lead to the same class. + +.. py:class:: inspect_type_links.first.Foo + + :ref:`first.Foo` and :ref:`inspect_type_links.first.Foo` should + lead to self; referencing the subclass via :ref:`Foo`, :ref:`Foo.Foo`, + :ref:`first.Foo.Foo` or :ref:`Bar`. :ref:`second.Foo` and + :ref:`inspect_type_links.Bar` lead to other classes. + + This is consistent with how Python type annotations inside *a class* are + interpreted -- see :ref:`reference_self_data`, :ref:`reference_inner_data` + and :ref:`reference_inner_other_data`. Inside *function definitions* the + rules are different as per https://docs.python.org/3/reference/executionmodel.html#resolution-of-names: + + The scope of names defined in a class block is limited to the class + block; it does not extend to the code blocks of methods + + This means relative annotations in :ref:`reference_self()`, + :ref:`reference_inner()` and :ref:`reference_inner_other()` are parsed + differently -- but in the documentation, these are shown with the same + rules as for data themselves. + +.. py:data:: inspect_type_links.first.Foo.reference_self_data + :summary: Referencing its wrapper class using ``first.Foo``, displayed + as ``first.Foo`` as well. ``Foo`` alone would reference the inner. This + is different from :ref:`reference_self()`. + +.. py:data:: inspect_type_links.first.Foo.reference_inner_data + :summary: Referencing the inner class using ``Foo``, ``Foo.Foo`` or + ``first.Foo.Foo``, displayed as just ``Foo``. This is different from + :ref:`reference_inner()`. + +.. py:data:: inspect_type_links.first.Foo.reference_inner_other_data + :summary: Referencing another inner class using ``Bar``, ``Foo.Bar`` or + ``first.Foo.Bar``, displayed as just ``Bar``. This is different from + :ref:`reference_inner_other()`. + +.. py:class:: inspect_type_links.first.Foo.Foo + + Referencing self as :ref:`Foo` or :ref:`Foo.Foo`, parent as + :ref:`first.Foo`, other as :ref:`second.Foo`. However inside annotations + ``Foo`` references the parent, consistently in a function and in data? + Am I doing something wrong? + +.. py:class:: inspect_type_links.first.sub.Foo + + Referencing self as :ref:`Foo` or :ref:`sub.Foo`, parent as + :ref:`first.Foo`, other as :ref:`second.Foo`. diff --git a/documentation/test_python/inspect_type_links/inspect_type_links.first.Foo.Foo.html b/documentation/test_python/inspect_type_links/inspect_type_links.first.Foo.Foo.html index 129c8253..7b00c5e0 100644 --- a/documentation/test_python/inspect_type_links/inspect_type_links.first.Foo.Foo.html +++ b/documentation/test_python/inspect_type_links/inspect_type_links.first.Foo.Foo.html @@ -30,23 +30,43 @@ Reference +

Referencing self as Foo or Foo.Foo, parent as +first.Foo, other as second.Foo. However inside annotations +Foo references the parent, consistently in a function and in data? +Am I doing something wrong?

Methods

def reference_parent(self, - a: first.Foo) + a: first.Foo, + b: first.Foo)
-
A method referencing its parent wrapper class
+
A method referencing its parent wrapper class using first.Foo. Foo works too, though. Weird. Displayed as first.Foo.
def reference_self(self, - a: Foo) + a: Foo, + b: Foo)
-
A method referencing its wrapper class
+
A method referencing its wrapper class using Foo.Foo or first.Foo.Foo, displayed as Foo in both cases; however using just Foo in the annotation references the parent?!
+
+
+
+

Data

+
+
+ reference_parent_data: typing.Tuple[first.Foo, first.Foo] = {} +
+
+
+ reference_self_data: typing.Tuple[Foo, Foo] = {} +
+
diff --git a/documentation/test_python/inspect_type_links/inspect_type_links.first.Foo.html b/documentation/test_python/inspect_type_links/inspect_type_links.first.Foo.html index aac4c36e..3e0e810d 100644 --- a/documentation/test_python/inspect_type_links/inspect_type_links.first.Foo.html +++ b/documentation/test_python/inspect_type_links/inspect_type_links.first.Foo.html @@ -31,13 +31,31 @@ +

first.Foo and inspect_type_links.first.Foo should +lead to self; referencing the subclass via Foo, Foo.Foo, +first.Foo.Foo or Bar. second.Foo and +inspect_type_links.Bar lead to other classes.

+

This is consistent with how Python type annotations inside a class are +interpreted -- see reference_self_data, reference_inner_data +and reference_inner_other_data. Inside function definitions the +rules are different as per https://docs.python.org/3/reference/executionmodel.html#resolution-of-names:

+
+The scope of names defined in a class block is limited to the class +block; it does not extend to the code blocks of methods
+

This means relative annotations in reference_self(), +reference_inner() and reference_inner_other() are parsed +differently -- but in the documentation, these are shown with the same +rules as for data themselves.

Classes

+
class Bar
+
Another inner class.
class Foo
An inner class in the first module
@@ -47,25 +65,55 @@
def reference_inner(self, - a: Foo) + a: Foo, + b: Foo)
-
A method referencing an inner class. This is quite a pathological case and I'm not sure if Foo or Foo.Foo is better.
+
Referencing an inner class using Foo.Foo and first.Foo.Foo. Outside of a function it would be enough to reference via Foo, thus docs display just Foo.
+
+ def reference_inner_other(self, + a: Bar) +
+
Referencing another inner class using Foo.Bar. Bar alone doesn't work, outside of a function it would, thus docs display just Bar.
def reference_other(self, - a: second.Foo) + a: second.Foo, + b: inspect_type_links.Bar) +
+
Referencing a type in another module using second.Foo or inspect_type_links.Bar.
+
+ def reference_parent(self, + a: inspect_type_links.Foo)
-
A method referencing a type in another module
+
Referencing a class in a parent module using inspect_type_links.Foo.
def reference_self(self, - a: first.Foo) + a: first.Foo, + b: first.Foo) +
+
Referencing its wrapper class using Foo and first.Foo. Outside of a function Foo would reference the inner, thus docs display first.Foo to disambiguate.
+
+
+
+

Data

+
+
+ reference_inner_data: typing.Tuple[Foo, Foo, Foo] = {} +
+
Referencing the inner class using Foo, Foo.Foo or +first.Foo.Foo, displayed as just Foo. This is different from +reference_inner().
+
+ reference_inner_other_data: typing.Tuple[Bar, Bar, Bar] = {}
-
A method referencing its wrapper class. Due to the inner Foo this is quite a pathological case and I'm not sure if first.Foo or Foo is better.
-
- def reference_sub(self, - a: sub.Foo, - b: sub.Foo) +
Referencing another inner class using Bar, Foo.Bar or +first.Foo.Bar, displayed as just Bar. This is different from +reference_inner_other().
+
+ reference_self_data: typing.Tuple[first.Foo] = {}
-
A method referencing a type in a submodule
+
Referencing its wrapper class using first.Foo, displayed +as first.Foo as well. Foo alone would reference the inner. This +is different from reference_self().
diff --git a/documentation/test_python/inspect_type_links/inspect_type_links.first.html b/documentation/test_python/inspect_type_links/inspect_type_links.first.html index 39bf563f..4ad7b7b5 100644 --- a/documentation/test_python/inspect_type_links/inspect_type_links.first.html +++ b/documentation/test_python/inspect_type_links/inspect_type_links.first.html @@ -36,6 +36,8 @@ +

Foo, first.Foo and inspect_type_links.first.Foo should +lead to the same class.

Modules

diff --git a/documentation/test_python/inspect_type_links/inspect_type_links.first.sub.Foo.html b/documentation/test_python/inspect_type_links/inspect_type_links.first.sub.Foo.html index 43f9dfcd..69fbb74f 100644 --- a/documentation/test_python/inspect_type_links/inspect_type_links.first.sub.Foo.html +++ b/documentation/test_python/inspect_type_links/inspect_type_links.first.sub.Foo.html @@ -34,6 +34,8 @@ +

Referencing self as Foo or sub.Foo, parent as +first.Foo, other as second.Foo.

Methods

diff --git a/documentation/test_python/inspect_type_links/inspect_type_links/__init__.py b/documentation/test_python/inspect_type_links/inspect_type_links/__init__.py index 35e9047a..09fe0bc3 100644 --- a/documentation/test_python/inspect_type_links/inspect_type_links/__init__.py +++ b/documentation/test_python/inspect_type_links/inspect_type_links/__init__.py @@ -1 +1,7 @@ from . import first, second + +class Foo: + """A class in the root module""" + +class Bar: + """Another class in the root module""" diff --git a/documentation/test_python/inspect_type_links/inspect_type_links/first/__init__.py b/documentation/test_python/inspect_type_links/inspect_type_links/first/__init__.py index b5a52c71..4a83acbf 100644 --- a/documentation/test_python/inspect_type_links/inspect_type_links/first/__init__.py +++ b/documentation/test_python/inspect_type_links/inspect_type_links/first/__init__.py @@ -1,28 +1,59 @@ """First module""" +from typing import Tuple + +import inspect_type_links from inspect_type_links import first from inspect_type_links import second class Foo: """A class in the first module""" - def reference_self(self, a: 'first.Foo'): - """A method referencing its wrapper class. Due to the inner Foo this is quite a pathological case and I'm not sure if first.Foo or Foo is better.""" + def reference_self(self, a: 'Foo', b: 'first.Foo'): + """Referencing its wrapper class using Foo and first.Foo. Outside of a function Foo would reference the inner, thus docs display first.Foo to disambiguate.""" + + assert Foo is first.Foo + + def reference_inner(self, a: 'Foo.Foo', b: 'first.Foo.Foo'): + """Referencing an inner class using Foo.Foo and first.Foo.Foo. Outside of a function it would be enough to reference via Foo, thus docs display just Foo.""" + + assert Foo.Foo is first.Foo.Foo - def reference_inner(self, a: 'first.Foo.Foo'): - """A method referencing an inner class. This is quite a pathological case and I'm not sure if Foo or Foo.Foo is better.""" + def reference_inner_other(self, a: 'Foo.Bar'): + """Referencing another inner class using Foo.Bar. Bar alone doesn't work, outside of a function it would, thus docs display just Bar.""" - def reference_other(self, a: second.Foo): - """A method referencing a type in another module""" + assert Foo.Bar is first.Foo.Bar + + def reference_parent(self, a: 'inspect_type_links.Foo'): + """Referencing a class in a parent module using inspect_type_links.Foo.""" + + def reference_other(self, a: second.Foo, b: 'inspect_type_links.Bar'): + """Referencing a type in another module using second.Foo or inspect_type_links.Bar.""" class Foo: """An inner class in the first module""" - def reference_self(self, a: 'first.Foo.Foo'): - """A method referencing its wrapper class""" + def reference_self(self, a: 'Foo.Foo', b: 'first.Foo.Foo'): + """A method referencing its wrapper class using Foo.Foo or first.Foo.Foo, displayed as Foo in both cases; however using just Foo in the annotation references the parent?!""" + + assert Foo.Foo is first.Foo.Foo + + def reference_parent(self, a: 'Foo', b: 'first.Foo'): + """A method referencing its parent wrapper class using first.Foo. Foo works too, though. Weird. Displayed as first.Foo.""" - def reference_parent(self, a: 'first.Foo'): - """A method referencing its parent wrapper class""" + assert Foo is first.Foo + + reference_self_data: Tuple['Foo.Foo', 'first.Foo.Foo'] = {} + reference_parent_data: Tuple['Foo', 'first.Foo'] = {} + + class Bar: + """Another inner class.""" + + reference_self_data: Tuple['first.Foo'] = {} + reference_inner_data: Tuple[Foo, 'Foo.Foo', 'first.Foo.Foo'] = {} + reference_inner_other_data: Tuple[Bar, 'Foo.Bar', 'first.Foo.Bar'] = {} + + #assert Foo is first.Foo.Foo def reference_self(a: Foo, b: first.Foo): """A function referencing a type in this module""" @@ -32,10 +63,12 @@ def reference_other(a: second.Foo): from . import sub -def _foo_reference_sub(self, a: sub.Foo, b: first.sub.Foo): - """A method referencing a type in a submodule""" - -setattr(Foo, 'reference_sub', _foo_reference_sub) - def reference_sub(a: sub.Foo, b: first.sub.Foo): """A function referencing a type in a submodule""" + +# Asserting on our assumptions +Foo().reference_self(None, None) +Foo().reference_inner(None, None) +Foo().reference_inner_other(None) +Foo.Foo().reference_self(None, None) +Foo.Foo().reference_parent(None, None) diff --git a/documentation/test_python/test_inspect.py b/documentation/test_python/test_inspect.py index 501b9de3..92b8f737 100644 --- a/documentation/test_python/test_inspect.py +++ b/documentation/test_python/test_inspect.py @@ -159,6 +159,7 @@ class TypeLinks(BaseInspectTestCase): def test(self): self.run_python({ 'PLUGINS': ['m.sphinx'], + 'INPUT_DOCS': ['docs.rst'], 'INPUT_PAGES': ['index.rst'], 'M_SPHINX_INVENTORIES': [ ('../../../doc/documentation/python.inv', 'https://docs.python.org/3/', [], ['m-doc-external'])] diff --git a/plugins/m/sphinx.py b/plugins/m/sphinx.py index 74a70033..310fa82f 100644 --- a/plugins/m/sphinx.py +++ b/plugins/m/sphinx.py @@ -38,6 +38,7 @@ from docutils.parsers.rst.states import Inliner from pelican import signals +referer_path = [] module_doc_output = None class_doc_output = None enum_doc_output = None @@ -217,9 +218,12 @@ def ref(name, rawtext, text, lineno, inliner: Inliner, options={}, content=[]): # Avoid assert on adding to undefined member later if 'classes' not in _options: _options['classes'] = [] - # Iterate through all prefixes, try to find the name + # Add prefixes of the referer path to the global prefix list, iterate + # through all of them, with names "closest" to the referer having a + # priority and try to find the name global intersphinx_inventory, intersphinx_name_prefixes - for prefix in intersphinx_name_prefixes: + prefixes = ['.'.join(referer_path[:len(referer_path) - i]) + '.' for i, _ in enumerate(referer_path)] + intersphinx_name_prefixes + for prefix in prefixes: found = None # If the target is prefixed with a type, try looking up that type @@ -285,6 +289,10 @@ def ref(name, rawtext, text, lineno, inliner: Inliner, options={}, content=[]): node = nodes.literal(rawtext, target, **_options) return [node], [] +def remember_referer_path(path): + global referer_path + referer_path = path + def merge_inventories(name_map, **kwargs): global intersphinx_inventory @@ -353,7 +361,7 @@ def merge_inventories(name_map, **kwargs): assert path not in data data[path] = value -def register_mcss(mcss_settings, module_doc_contents, class_doc_contents, enum_doc_contents, function_doc_contents, property_doc_contents, data_doc_contents, hooks_post_crawl, **kwargs): +def register_mcss(mcss_settings, module_doc_contents, class_doc_contents, enum_doc_contents, function_doc_contents, property_doc_contents, data_doc_contents, hooks_post_crawl, hooks_pre_page, **kwargs): global module_doc_output, class_doc_output, enum_doc_output, function_doc_output, property_doc_output, data_doc_output module_doc_output = module_doc_contents class_doc_output = class_doc_contents @@ -374,6 +382,7 @@ def register_mcss(mcss_settings, module_doc_contents, class_doc_contents, enum_d rst.roles.register_local_role('ref', ref) + hooks_pre_page += [remember_referer_path] hooks_post_crawl += [merge_inventories] def _pelican_configure(pelicanobj): -- 2.30.2