From d034768fa9454208c7c7c24666b70ef66f5c1f46 Mon Sep 17 00:00:00 2001 From: Mark Daoust Date: Thu, 6 Sep 2018 11:22:20 -0700 Subject: [PATCH] Fix link generator for module level constants. Moved _is_free_function to parser.is_free_function Merged the `is_class` and `is_module` properties into `is_fragment`, since this is the only thing they were being used for. With the additions to `pretty_docs.py`, all documented objects either have a page to them self, or a `#id` fragment on their parents page, the `is_fragment` property indicates which. In all uses of `documentation_path`, except the "reference_to_url" it's safe to assume that `is_fragment` is `False` (this is the current correct behavior). fixes #20913 PiperOrigin-RevId: 211838909 --- tensorflow/tools/docs/generate_lib.py | 19 +----- tensorflow/tools/docs/parser.py | 85 +++++++++++++++----------- tensorflow/tools/docs/parser_test.py | 86 +++++++++++++++++---------- tensorflow/tools/docs/pretty_docs.py | 3 +- 4 files changed, 107 insertions(+), 86 deletions(-) diff --git a/tensorflow/tools/docs/generate_lib.py b/tensorflow/tools/docs/generate_lib.py index 7db89f7d24c..1cd9cb7ca92 100644 --- a/tensorflow/tools/docs/generate_lib.py +++ b/tensorflow/tools/docs/generate_lib.py @@ -36,23 +36,6 @@ from tensorflow.tools.docs import pretty_docs from tensorflow.tools.docs import py_guide_parser -def _is_free_function(py_object, full_name, index): - """Check if input is a free function (and not a class- or static method).""" - if not tf_inspect.isfunction(py_object): - return False - - # Static methods are functions to tf_inspect (in 2.7), so check if the parent - # is a class. If there is no parent, it's not a function. - if '.' not in full_name: - return False - - parent_name = full_name.rsplit('.', 1)[0] - if tf_inspect.isclass(index[parent_name]): - return False - - return True - - def write_docs(output_dir, parser_config, yaml_toc, @@ -109,7 +92,7 @@ def write_docs(output_dir, # Methods and some routines are documented only as part of their class. if not (tf_inspect.ismodule(py_object) or tf_inspect.isclass(py_object) or - _is_free_function(py_object, full_name, parser_config.index)): + parser.is_free_function(py_object, full_name, parser_config.index)): continue sitepath = os.path.join('api_docs/python', diff --git a/tensorflow/tools/docs/parser.py b/tensorflow/tools/docs/parser.py index 4afb61e3654..a6159fa6927 100644 --- a/tensorflow/tools/docs/parser.py +++ b/tensorflow/tools/docs/parser.py @@ -35,6 +35,28 @@ from tensorflow.python.util import tf_inspect from tensorflow.tools.docs import doc_controls +def is_free_function(py_object, full_name, index): + """Check if input is a free function (and not a class- or static method). + + Args: + py_object: The the object in question. + full_name: The full name of the object, like `tf.module.symbol`. + index: The {full_name:py_object} dictionary for the public API. + + Returns: + True if the obeject is a stand-alone function, and not part of a class + definition. + """ + if not tf_inspect.isfunction(py_object): + return False + + parent_name = full_name.rsplit('.', 1)[0] + if tf_inspect.isclass(index[parent_name]): + return False + + return True + + # A regular expression capturing a python identifier. IDENTIFIER_RE = r'[a-zA-Z_]\w*' @@ -74,7 +96,7 @@ class _Errors(object): return self._errors == other._errors # pylint: disable=protected-access -def documentation_path(full_name): +def documentation_path(full_name, is_fragment=False): """Returns the file path for the documentation for the given API symbol. Given the fully qualified name of a library symbol, compute the path to which @@ -84,12 +106,22 @@ def documentation_path(full_name): Args: full_name: Fully qualified name of a library symbol. - + is_fragment: If `False` produce a direct markdown link (`tf.a.b.c` --> + `tf/a/b/c.md`). If `True` produce fragment link, `tf.a.b.c` --> + `tf/a/b.md#c` Returns: The file path to which to write the documentation for `full_name`. """ - dirs = full_name.split('.') - return os.path.join(*dirs) + '.md' + parts = full_name.split('.') + if is_fragment: + parts, fragment = parts[:-1], parts[-1] + + result = os.path.join(*parts) + '.md' + + if is_fragment: + result = result + '#' + fragment + + return result def _get_raw_docstring(py_object): @@ -136,8 +168,7 @@ class ReferenceResolver(object): doc. """ - def __init__(self, duplicate_of, doc_index, is_class, is_module, - py_module_names): + def __init__(self, duplicate_of, doc_index, is_fragment, py_module_names): """Initializes a Reference Resolver. Args: @@ -145,16 +176,15 @@ class ReferenceResolver(object): symbols. doc_index: A `dict` mapping symbol name strings to objects with `url` and `title` fields. Used to resolve @{$doc} references in docstrings. - is_class: A map from full names to bool for each symbol. - is_module: A map from full names to bool for each symbol. + is_fragment: A map from full names to bool for each symbol. If True the + object lives at a page fragment `tf.a.b.c` --> `tf/a/b#c`. If False + object has a page to itself: `tf.a.b.c` --> `tf/a/b/c`. py_module_names: A list of string names of Python modules. """ self._duplicate_of = duplicate_of self._doc_index = doc_index - self._is_class = is_class - self._is_module = is_module - - self._all_names = set(is_class.keys()) + self._is_fragment = is_fragment + self._all_names = set(is_fragment.keys()) self._py_module_names = py_module_names self.current_doc_full_name = None @@ -181,21 +211,18 @@ class ReferenceResolver(object): Returns: an instance of `ReferenceResolver` () """ - is_class = { - name: tf_inspect.isclass(visitor.index[name]) - for name, obj in visitor.index.items() - } + is_fragment = {} + for name, obj in visitor.index.items(): + has_page = ( + tf_inspect.isclass(obj) or tf_inspect.ismodule(obj) or + is_free_function(obj, name, visitor.index)) - is_module = { - name: tf_inspect.ismodule(visitor.index[name]) - for name, obj in visitor.index.items() - } + is_fragment[name] = not has_page return cls( duplicate_of=visitor.duplicate_of, doc_index=doc_index, - is_class=is_class, - is_module=is_module, + is_fragment=is_fragment, **kwargs) @classmethod @@ -344,19 +371,7 @@ class ReferenceResolver(object): raise TFDocsError( 'Cannot make link to "%s": Not in index.' % master_name) - # If this is a member of a class, link to the class page with an anchor. - ref_path = None - if not (self._is_class[master_name] or self._is_module[master_name]): - idents = master_name.split('.') - if len(idents) > 1: - class_name = '.'.join(idents[:-1]) - assert class_name in self._all_names - if self._is_class[class_name]: - ref_path = documentation_path(class_name) + '#%s' % idents[-1] - - if not ref_path: - ref_path = documentation_path(master_name) - + ref_path = documentation_path(master_name, self._is_fragment[master_name]) return os.path.join(relative_path_to_root, ref_path) def _one_ref(self, match, relative_path_to_root): diff --git a/tensorflow/tools/docs/parser_test.py b/tensorflow/tools/docs/parser_test.py index 71e96afa105..8a41796fb9f 100644 --- a/tensorflow/tools/docs/parser_test.py +++ b/tensorflow/tools/docs/parser_test.py @@ -28,6 +28,12 @@ from tensorflow.python.util import tf_inspect from tensorflow.tools.docs import doc_controls from tensorflow.tools.docs import parser +# The test needs a real module. `types.ModuleType()` doesn't work, as the result +# is a `builtin` module. Using "parser" here is arbitraty. The tests don't +# depend on the module contents. At this point in the process the public api +# has already been extracted. +test_module = parser + def test_function(unused_arg, unused_kwarg='default'): """Docstring for test function.""" @@ -334,15 +340,16 @@ class ParserTest(googletest.TestCase): self.assertEqual('my_method', page_info.methods[0].short_name) def test_docs_for_module(self): - # Get the current module. - module = sys.modules[__name__] index = { - 'TestModule': module, - 'TestModule.test_function': test_function, + 'TestModule': + test_module, + 'TestModule.test_function': + test_function, 'TestModule.test_function_with_args_kwargs': - test_function_with_args_kwargs, - 'TestModule.TestClass': TestClass, + test_function_with_args_kwargs, + 'TestModule.TestClass': + TestClass, } visitor = DummyVisitor(index=index, duplicate_of={}) @@ -365,11 +372,13 @@ class ParserTest(googletest.TestCase): base_dir='/') page_info = parser.docs_for_object( - full_name='TestModule', py_object=module, parser_config=parser_config) + full_name='TestModule', + py_object=test_module, + parser_config=parser_config) # Make sure the brief docstring is present - self.assertEqual(tf_inspect.getdoc(module).split('\n')[0], - page_info.doc.brief) + self.assertEqual( + tf_inspect.getdoc(test_module).split('\n')[0], page_info.doc.brief) # Make sure that the members are there funcs = {f_info.obj for f_info in page_info.functions} @@ -378,8 +387,9 @@ class ParserTest(googletest.TestCase): classes = {cls_info.obj for cls_info in page_info.classes} self.assertEqual({TestClass}, classes) - # Make sure this file is contained as the definition location. - self.assertEqual(os.path.relpath(__file__, '/'), page_info.defined_in.path) + # Make sure the module's file is contained as the definition location. + self.assertEqual( + os.path.relpath(test_module.__file__, '/'), page_info.defined_in.path) def test_docs_for_function(self): index = { @@ -495,6 +505,7 @@ class ParserTest(googletest.TestCase): duplicate_of = {'tf.third': 'tf.fourth'} index = { + 'tf': test_module, 'tf.fancy': test_function_with_fancy_docstring, 'tf.reference': HasOneMember, 'tf.reference.foo': HasOneMember.foo, @@ -521,20 +532,18 @@ class ParserTest(googletest.TestCase): 'NumPy has nothing as awesome as this function.\n') def test_generate_index(self): - module = sys.modules[__name__] index = { - 'TestModule': module, - 'test_function': test_function, - 'TestModule.test_function': test_function, - 'TestModule.TestClass': TestClass, - 'TestModule.TestClass.a_method': TestClass.a_method, - 'TestModule.TestClass.a_property': TestClass.a_property, - 'TestModule.TestClass.ChildClass': TestClass.ChildClass, - } - duplicate_of = { - 'TestModule.test_function': 'test_function' + 'tf': test_module, + 'tf.TestModule': test_module, + 'tf.test_function': test_function, + 'tf.TestModule.test_function': test_function, + 'tf.TestModule.TestClass': TestClass, + 'tf.TestModule.TestClass.a_method': TestClass.a_method, + 'tf.TestModule.TestClass.a_property': TestClass.a_property, + 'tf.TestModule.TestClass.ChildClass': TestClass.ChildClass, } + duplicate_of = {'tf.TestModule.test_function': 'tf.test_function'} visitor = DummyVisitor(index=index, duplicate_of=duplicate_of) @@ -553,7 +562,7 @@ class ParserTest(googletest.TestCase): self.assertIn('TestModule.test_function', docs) # Leading backtick to make sure it's included top-level. # This depends on formatting, but should be stable. - self.assertIn('test_function', docs) + self.assertIn('tf.test_function', docs) def test_argspec_for_functools_partial(self): # pylint: disable=unused-argument @@ -665,22 +674,18 @@ class ParserTest(googletest.TestCase): duplicate_of = {'AClass': ['AClass2']} doc_index = {'doc': you_cant_serialize_this} - is_class = { + is_fragment = { 'tf': False, - 'tf.AClass': True, - 'tf.AClass2': True, - 'tf.function': False - } - is_module = { - 'tf': True, + 'tf.VERSION': True, 'tf.AClass': False, + 'tf.AClass.method': True, 'tf.AClass2': False, 'tf.function': False } py_module_names = ['tf', 'tfdbg'] - resolver = parser.ReferenceResolver(duplicate_of, doc_index, is_class, - is_module, py_module_names) + resolver = parser.ReferenceResolver(duplicate_of, doc_index, is_fragment, + py_module_names) outdir = googletest.GetTempDir() @@ -692,6 +697,23 @@ class ParserTest(googletest.TestCase): # There are no __slots__, so all fields are visible in __dict__. self.assertEqual(resolver.__dict__, resolver2.__dict__) + def testIsFreeFunction(self): + + result = parser.is_free_function(test_function, 'test_module.test_function', + {'test_module': test_module}) + self.assertTrue(result) + + result = parser.is_free_function(test_function, 'TestClass.test_function', + {'TestClass': TestClass}) + self.assertFalse(result) + + result = parser.is_free_function(TestClass, 'TestClass', {}) + self.assertFalse(result) + + result = parser.is_free_function(test_module, 'test_module', {}) + self.assertFalse(result) + + RELU_DOC = """Computes rectified linear: `max(features, 0)` Args: diff --git a/tensorflow/tools/docs/pretty_docs.py b/tensorflow/tools/docs/pretty_docs.py index 448f246e0ea..1a3e79621f8 100644 --- a/tensorflow/tools/docs/pretty_docs.py +++ b/tensorflow/tools/docs/pretty_docs.py @@ -255,8 +255,9 @@ def _build_module_page(page_info): # at least for basic types. parts.append('## Other Members\n\n') + h3 = '

{short_name}

\n\n' for item in page_info.other_members: - parts.append('`{short_name}`\n\n'.format(**item._asdict())) + parts.append(h3.format(**item._asdict())) return ''.join(parts)