Improvements to pydocstyle Python parsing

* Moves relative path parsing away from the base mapper implementation
* Change argument parsing from splitting first line of source with ',' to use
  AST traversal instead. This is not complete, but mostly PoC for now. Full
  traversal into argument type nodes will allow us to get nested dict() etc.
  We should open a ticket to track this work
* Cleans up some of the templates to reduce duplicate titles
* Adds a directive for nesting rST from constructs that might have headings.
  Remove the first heading in this case to address the case where a module has a
  docstring with a heading up front
* Adds tests
* Replaces example module with module that has more failing cases of parsing

Closes #78
Fixes #80
Fixes #81
Fixes #82
Fixes #83
Fixes #84
Fixes #85

autoapi/directives.py

@@ -0,0 +1,35 @@
+"""AutoAPI directives"""
+
+from docutils.parsers.rst import Directive
+from docutils import nodes
+
+from sphinx.util.nodes import nested_parse_with_titles
+
+
+class NestedParse(Directive):
+
+    """Nested parsing to remove the first heading of included rST
+
+    This is used to handle the case where we like to remove user supplied
+    headings from module docstrings. This is required to reduce the number of
+    duplicate headings on sections.
+    """
+
+    has_content = 1
+    required_arguments = 0
+    optional_arguments = 0
+    final_argument_whitespace = False
+    option_spec = {}
+
+    def run(self):
+        node = nodes.paragraph()
+        node.document = self.state.document
+        nested_parse_with_titles(self.state, self.content, node)
+        try:
+            title_node = node[0][0]
+            if isinstance(title_node, nodes.title):
+                if isinstance(title_node[0], nodes.Text):
+                    del node[0][0][0]
+        except IndexError:
+            pass
+        return [node]

autoapi/extension.py

@@ -10,8 +10,10 @@ import shutil
 from sphinx.util.console import darkgreen, bold
 from sphinx.addnodes import toctree
 from sphinx.errors import ExtensionError
+from docutils.parsers.rst import directives
 
 from .backends import default_file_mapping, default_ignore_patterns, default_backend_mapping
+from .directives import NestedParse
 from .settings import API_ROOT
 
 default_options = ['members', 'undoc-members', 'private-members', 'special-members']
@@ -75,7 +77,6 @@ def run_autoapi(app):
         out_suffix = app.config.source_suffix[0]
 
     # Actual meat of the run.
-
     app.info(bold('[AutoAPI] ') + darkgreen('Loading Data'))
     domain_obj.load(
         patterns=file_patterns,
@@ -140,3 +141,4 @@ def setup(app):
     app.add_config_value('autoapi_add_toctree_entry', True, 'html')
     app.add_config_value('autoapi_template_dir', [], 'html')
     app.add_stylesheet('autoapi.css')
+    directives.register_directive('autoapi-nested-parse', NestedParse)

autoapi/mappers/base.py

@@ -41,7 +41,6 @@ class PythonMapperBase(object):
     :var list children: Children of this object
     :var list parameters: Parameters to this object
     :var list methods: Methods on this object
-
     '''
 
     language = 'base'
@@ -49,9 +48,8 @@ class PythonMapperBase(object):
     # Create a page in the output for this object.
     top_level_object = False
 
-    def __init__(self, obj, path, options=None, jinja_env=None, url_root=None):
+    def __init__(self, obj, options=None, jinja_env=None, url_root=None):
         self.obj = obj
-        self.path = path
         self.options = options
         if jinja_env:
             self.jinja_env = jinja_env
@@ -125,11 +123,9 @@ class PythonMapperBase(object):
 
     def include_dir(self, root):
         """Return directory of file"""
-        return os.path.join(
+        parts = [root]
-            root,
+        parts.extend(self.pathname.split(os.path.sep))
-            os.path.dirname(self.path.relative),
+        return '/'.join(parts)
-            self.pathname,
-        )
 
     @property
     def include_path(self):
@@ -199,7 +195,7 @@ class SphinxMapperBase(object):
 
         '''
         for path in self.find_files(patterns=patterns, dirs=dirs, ignore=ignore):
-            data = self.read_file(path=path.absolute)
+            data = self.read_file(path=path)
             if data:
                 self.paths[path] = data
 
@@ -227,14 +223,10 @@ class SphinxMapperBase(object):
                             continue
 
                         # Make sure the path is full
-                        if os.path.isabs(filename):
+                        if not os.path.isabs(filename):
-                            ret_path = filename
+                            filename = os.path.join(root, filename)
-                        else:
-                            ret_path = os.path.join(root, filename)
 
-                        rel_path = ret_path.replace(_dir, '')
+                        files_to_read.append(filename)
-                        path_obj = Path(ret_path, rel_path[1:])
-                        files_to_read.append(path_obj)
 
         for _path in self.app.status_iterator(
                 files_to_read,
@@ -266,7 +258,7 @@ class SphinxMapperBase(object):
             for obj in self.create_class(data, options=options, path=path):
                 self.add_object(obj)
 
-    def create_class(self, obj, options=None, **kwargs):
+    def create_class(self, obj, options=None, path=None, **kwargs):
         '''
         Create class object.
 

autoapi/mappers/python.py

@@ -1,88 +1,92 @@
+import sys
 import os
+import re
 import textwrap
+import ast
 from collections import defaultdict
 from pydocstyle.parser import Parser
 
+if sys.version_info < (3,):
+    from itertools import izip_longest as zip_longest
+else:
+    from itertools import zip_longest
+
 from .base import PythonMapperBase, SphinxMapperBase
 from ..utils import slugify
 
 
 class PythonSphinxMapper(SphinxMapperBase):
 
-    '''Auto API domain handler for Python
+    """Auto API domain handler for Python
 
     Parses directly from Python files.
 
     :param app: Sphinx application passed in as part of the extension
-    '''
+    """
+
+    def load(self, patterns, dirs, **kwargs):
+        """Load objects from the filesystem into the ``paths`` dictionary
+
+        Also include an attribute on the object, ``relative_path`` which is the
+        shortened, relative path the package/module
+        """
+        for dir_ in dirs:
+            for path in self.find_files(patterns=patterns, dirs=[dir_], **kwargs):
+                data = self.read_file(path=path)
+                data.relative_path = os.path.relpath(path, dir_)
+                if data:
+                    self.paths[path] = data
 
     def read_file(self, path, **kwargs):
-        '''Read file input into memory, returning deserialized objects
+        """Read file input into memory, returning deserialized objects
 
         :param path: Path of file to read
-        '''
+        """
-
         try:
             parsed_data = Parser()(open(path), path)
             return parsed_data
-        except IOError:
+        except (IOError, TypeError, ImportError):
-            self.app.warn('Error reading file: {0}'.format(path))
-        except TypeError:
-            self.app.warn('Error reading file: {0}'.format(path))
-        except ImportError:
             self.app.warn('Error reading file: {0}'.format(path))
         return None
 
-    def create_class(self, data, options=None, **kwargs):
+    def create_class(self, data, options=None, path=None, **kwargs):
         """Create a class from the passed in data
 
         :param data: dictionary data of pydocstyle output
         """
         obj_map = dict((cls.type, cls) for cls
-                       in [PythonClass, PythonFunction, PythonModule, PythonMethod, PythonPackage])
+                       in [PythonClass, PythonFunction, PythonModule,
+                           PythonMethod, PythonPackage])
         try:
             cls = obj_map[data.kind]
         except KeyError:
-            self.app.warn("Unknown Type: %s" % data.kind)
+            self.app.warn("Unknown type: %s" % data.kind)
         else:
-            path = kwargs.get('path')
             obj = cls(data, jinja_env=self.jinja_env,
-                      options=self.app.config.autoapi_options, path=path
+                      options=self.app.config.autoapi_options, **kwargs)
-                      )
             for child_data in data.children:
-                for child_obj in self.create_class(child_data, options=options, path=path):
+                for child_obj in self.create_class(child_data, options=options,
+                                                   **kwargs):
                     obj.children.append(child_obj)
-                    self.add_object(child_obj)
             yield obj
 
 
 class PythonPythonMapper(PythonMapperBase):
 
     language = 'python'
+    is_callable = False
 
     def __init__(self, obj, **kwargs):
         super(PythonPythonMapper, self).__init__(obj, **kwargs)
 
-        # Properly name the object with dot notation
+        self.name = self._get_full_name(obj)
-        if self.top_level_object:
+        self.id = slugify(self.name)
-            name = self.path.relative.split('.')[0].replace('/', '.')
-        else:
-            name = '.'.join([
-                os.path.dirname(self.path.relative).replace('/', '.'),
-                obj.name
-            ])
-        self.id = slugify(name)
-        self.name = name
 
         # Optional
         self.children = []
-        try:
+        self.args = []
-            args = obj.source.split('\n')[0]
+        if self.is_callable:
-            args = args.split('(')[1]
+            self.args = self._get_arguments(obj)
-            args = args.split(')')[0]
-            self.args = args.split(',')
-        except:
-            args = ''
         self.docstring = obj.docstring or ''
         self.docstring = textwrap.dedent(self.docstring)
         self.docstring = self.docstring.replace("'''", '').replace('"""', '')
@@ -95,34 +99,151 @@ class PythonPythonMapper(PythonMapperBase):
         self.item_map = defaultdict(list)
 
     @property
-    def undoc_member(self):
+    def is_undoc_member(self):
         return self.docstring == ''
 
     @property
-    def private_member(self):
+    def is_private_member(self):
         return self.short_name[0] == '_'
 
     @property
-    def special_member(self):
+    def is_special_member(self):
         return self.short_name[0:2] == '__'
 
     @property
     def display(self):
-        if self.undoc_member and 'undoc-members' not in self.options:
+        if self.is_undoc_member and 'undoc-members' not in self.options:
             return False
-        if self.private_member and 'private-members' not in self.options:
+        if self.is_private_member and 'private-members' not in self.options:
             return False
-        if self.special_member and 'special-members' not in self.options:
+        if self.is_special_member and 'special-members' not in self.options:
             return False
         return True
 
+    @staticmethod
+    def _get_full_name(obj):
+        """Recursively build the full name of the object from pydocstyle
+
+        Uses an additional attribute added to the object, ``relative_path``.
+        This is the shortened path of the object name, if the object is a
+        package or module.
+
+        :param obj: pydocstyle object, as returned from Parser()
+        :returns: Dotted name of object
+        :rtype: str
+        """
+
+        def _inner(obj, parts=[]):
+            obj_kind = obj.kind
+            obj_name = obj.name
+            if obj_kind == 'module':
+                obj_name = getattr(obj, 'relative_path', None) or obj.name
+                obj_name = obj_name.replace('/', '.')
+                ext = '.py'
+                if obj_name.endswith(ext):
+                    obj_name = obj_name[:-len(ext)]
+            elif obj_kind == 'package':
+                obj_name = getattr(obj, 'relative_path', None) or obj.name
+                exts = ['/__init__.py', '.py']
+                for ext in exts:
+                    if obj_name.endswith(ext):
+                        obj_name = obj_name[:-len(ext)]
+                obj_name = obj_name.split('/').pop()
+            parts.insert(0, obj_name)
+            try:
+                return _inner(obj.parent, parts)
+            except AttributeError:
+                pass
+            return parts
+
+        return '.'.join(_inner(obj))
+
+    @staticmethod
+    def _get_arguments(obj):
+        """Get arguments from a pydocstyle object
+
+        :param obj: pydocstyle object, as returned from Parser()
+        :returns: list of argument or argument and value pairs
+        :rtype: list
+        """
+        arguments = []
+        source = textwrap.dedent(obj.source)
+        # Bare except here because AST parsing can throw any number of
+        # exceptions, including SyntaxError
+        try:
+            parsed = ast.parse(source)
+        except:  # noqa
+            return
+        parsed_args = parsed.body[0].args
+        arg_names = [arg.id if sys.version_info < (3,) else arg.arg
+                     for arg in parsed_args.args]
+
+        # Get defaults for display based on AST node type
+        arg_defaults = []
+        pydocstyle_map = {
+            ast.Name: 'id',
+            ast.Num: 'n',
+            ast.Str: lambda obj: '"{0}"'.format(obj.s),
+            ast.Call: lambda obj: obj.func.id,
+            # TODO these require traversal into the AST nodes. Add this for more
+            # complete argument parsing, or handle with a custom AST traversal.
+            ast.List: lambda _: 'list',
+            ast.Tuple: lambda _: 'tuple',
+            ast.Set: lambda _: 'set',
+            ast.Dict: lambda _: 'dict',
+        }
+        if sys.version_info >= (3,):
+            pydocstyle_map.update({
+                ast.NameConstant: 'value',
+            })
+
+        for value in parsed_args.defaults:
+            default = None
+            try:
+                default = pydocstyle_map[type(value)](value)
+            except TypeError:
+                default = getattr(value, pydocstyle_map[type(value)])
+            except KeyError:
+                pass
+            if default is None:
+                default = 'None'
+            arg_defaults.append(default)
+
+        # Apply defaults padded to the end of the longest list. AST returns
+        # argument defaults as a short array that applies to the end of the list
+        # of arguments
+        for (name, default) in zip_longest(reversed(arg_names),
+                                           reversed(arg_defaults)):
+            arg = name
+            if default is not None:
+                arg = '{0}={1}'.format(name, default)
+            arguments.insert(0, arg)
+
+        # Add *args and **kwargs
+        if parsed_args.vararg:
+            arguments.append('*{0}'.format(
+                parsed_args.vararg
+                if sys.version_info < (3,3)
+                else parsed_args.vararg.arg
+            ))
+        if parsed_args.kwarg:
+            arguments.append('**{0}'.format(
+                parsed_args.kwarg
+                if sys.version_info < (3,3)
+                else parsed_args.kwarg.arg
+            ))
+
+        return arguments
+
 
 class PythonFunction(PythonPythonMapper):
     type = 'function'
+    is_callable = True
 
 
 class PythonMethod(PythonPythonMapper):
     type = 'method'
+    is_callable = True
 
 
 class PythonModule(PythonPythonMapper):

autoapi/templates/python/class.rst

@@ -1,13 +1,13 @@
-{% if not 'nested' in obj._human %}
+.. autoapi-hidden::
-Class {{ obj.short_name }}
+   {{ obj.short_name }}
-~~~~~~{{ "~" * obj.short_name|length }}
+   {{ "=" * obj.short_name|length }}
-{% endif %}
 
 .. py:class:: {{ obj.short_name }}{% if obj.args %}({{ obj.args|join(',') }}){% endif %}
 
    {%- if obj.docstring %}
 
-   {{ obj.docstring|indent(3) }}
+   .. autoapi-nested-parse::
+      {{ obj.docstring|indent(6) }}
 
    {% endif %}
 

autoapi/templates/python/member.rst

@@ -2,4 +2,3 @@
 
    {{ obj.docstring|indent(3) }}
 
-

autoapi/templates/python/method.rst

@@ -3,7 +3,7 @@
 .. method:: {{ obj.name }}({{ obj.args[1:]|join(',') }})
 
    {% if obj.docstring %}
-   {{ obj.docstring }}
+   {{ obj.docstring|indent(3) }}
    {% endif %}
 
 {% endif %}

autoapi/templates/python/module.rst

@@ -1,14 +1,15 @@
-Module {{ obj.name }}
+{{ obj.name }}
--------{{ "-" * obj.name|length }}
+{{ "=" * obj.name|length }}
+
+.. py:module:: {{ obj.name }}
 
 {%- if obj.docstring %}
 
-{{ obj.docstring }}
+.. autoapi-nested-parse::
+   {{ obj.docstring|indent(3) }}
 
 {% endif %}
 
-.. py:module:: {{ obj.name }}
-
 {% block content %}
 {%- for obj_item in obj.children %}
 
@@ -16,4 +17,3 @@ Module {{ obj.name }}
 
 {%- endfor %}
 {% endblock %}
-

autoapi/templates/python/package.rst

@@ -1,19 +1 @@
-Package {{ obj.name }}
+{% extends "python/module.rst" %}
-========{{ "=" * obj.name|length }}
-
-{%- if obj.docstring %}
-
-{{ obj.docstring }}
-
-{% endif %}
-
-.. py:module:: {{ obj.name }}
-
-{% block content %}
-{%- for obj_item in obj.children %}
-
-{{ obj_item.rendered|indent(0) }}
-
-{%- endfor %}
-{% endblock %}
-

tests/test_integration.py

@@ -3,15 +3,15 @@ import os
 import sys
 import shutil
 import unittest
+from contextlib import contextmanager
 
 from mock import patch
 
 from sphinx.application import Sphinx
 
 
-class LanguageIntegrationTests(unittest.TestCase):
+@contextmanager
-
+def sphinx_build(test_dir):
-    def _run_test(self, test_dir, test_file, test_string):
     os.chdir('tests/{0}'.format(test_dir))
     try:
         app = Sphinx(
@@ -22,14 +22,21 @@ class LanguageIntegrationTests(unittest.TestCase):
             buildername='text',
         )
         app.build(force_all=True)
-            with open(test_file) as fin:
+        yield
-                text = fin.read().strip()
-                self.assertIn(test_string, text)
     finally:
         shutil.rmtree('_build')
         os.chdir('../..')
 
 
+class LanguageIntegrationTests(unittest.TestCase):
+
+    def _run_test(self, test_dir, test_file, test_string):
+        with sphinx_build(test_dir):
+            with open(test_file) as fin:
+                text = fin.read().strip()
+                self.assertIn(test_string, text)
+
+
 class JavaScriptTests(LanguageIntegrationTests):
 
     def _js_read(self, path):
@@ -60,12 +67,27 @@ class GoTests(LanguageIntegrationTests):
 
 class PythonTests(LanguageIntegrationTests):
 
-    @unittest.skipIf(sys.version_info > (3, 0), 'Epydoc does not support Python 3')
     def test_integration(self):
-        self._run_test(
+        with sphinx_build('pyexample'):
-            'pyexample',
+            example_file = open('_build/text/autoapi/example/index.txt').read()
-            '_build/text/autoapi/example/index.txt',
+            self.assertIn(
-            'Compute the square root of x and return it'
+                'class example.Foo',
+                example_file
+            )
+            self.assertIn(
+                'example.Foo.method_okay(foo=None, bar=None)',
+                example_file
+            )
+            self.assertIn(
+                'example.Foo.method_multiline(foo=None, bar=None, baz=None)',
+                example_file
+            )
+            self.assertIn(
+                'example.Foo.method_tricky(foo=None, bar=dict)',
+                example_file
+            )
+            self.assertFalse(
+                os.path.exists('_build/text/autoapi/method_multiline')
             )
 
 
@@ -96,7 +118,6 @@ class DotNetTests(LanguageIntegrationTests):
 
 class IntegrationTests(LanguageIntegrationTests):
 
-    @unittest.skipIf(sys.version_info > (3, 0), 'Epydoc does not support Python 3')
     def test_template_overrides(self):
         self._run_test(
             'templateexample',
@@ -107,7 +128,6 @@ class IntegrationTests(LanguageIntegrationTests):
 
 class TOCTreeTests(LanguageIntegrationTests):
 
-    @unittest.skipIf(sys.version_info > (3, 0), 'Epydoc does not support Python 3')
     def test_toctree_overrides(self):
         self._run_test(
             'toctreeexample',

tests/test_objects.py

@@ -4,10 +4,12 @@
 
 import os
 import unittest
+from collections import namedtuple
 
 from jinja2 import Environment, FileSystemLoader
 
 from autoapi.mappers import dotnet
+from autoapi.mappers import python
 from autoapi.settings import TEMPLATE_DIR
 
 
@@ -135,3 +137,93 @@ class DotNetObjectTests(unittest.TestCase):
         self.assertEqual(cls.include_path, '/autoapi/Foo/Bar/Widget/index')
         cls = dotnet.DotNetClass({'id': 'Foo.Bar.Widget'}, url_root='/autofoo')
         self.assertEqual(cls.include_path, '/autofoo/Foo/Bar/Widget/index')
+
+
+class PythonObjectTests(unittest.TestCase):
+
+    def test_full_name(self):
+        """Full name resolution on nested objects"""
+        Source = namedtuple('Source', ['kind', 'name', 'parent'])
+
+        obj_module = Source(kind='module', name='example/example.py', parent=None)
+        obj_class = Source(kind='class', name='Foo', parent=obj_module)
+        obj_method = Source(kind='method', name='bar', parent=obj_class)
+
+        self.assertEqual(
+            python.PythonPythonMapper._get_full_name(obj_module),
+            'example.example'
+        )
+        self.assertEqual(
+            python.PythonPythonMapper._get_full_name(obj_class),
+            'example.example.Foo'
+        )
+        self.assertEqual(
+            python.PythonPythonMapper._get_full_name(obj_method),
+            'example.example.Foo.bar'
+        )
+
+    def test_arguments(self):
+        """Argument parsing of source"""
+        Source = namedtuple('Source', ['source', 'docstring'])
+
+        obj = Source(
+            source=('def foobar(self, bar, baz=42, foo=True,\n'
+                    '           *args, **kwargs):\n'
+                    '    "This is a docstring"\n'
+                    '    return True\n'),
+            docstring='"This is a docstring"',
+        )
+
+        self.assertEqual(
+            python.PythonPythonMapper._get_arguments(obj),
+            ['self', 'bar', 'baz=42', 'foo=True', '*args', '**kwargs']
+        )
+
+    def test_advanced_arguments(self):
+        """Advanced argument parsing"""
+        Source = namedtuple('Source', ['source', 'docstring'])
+
+        obj = Source(
+            source=('def foobar(self, a, b, c=42, d="string", e=(1,2),\n'
+                    '           f={"a": True}, g=None, h=[1,2,3,4],\n'
+                    '           i=dict(a=True), j=False, *args, **kwargs):\n'
+                    '    "This is a docstring"\n'
+                    '    return True\n'),
+            docstring='"This is a docstring"',
+        )
+
+        self.assertEqual(
+            python.PythonPythonMapper._get_arguments(obj),
+            [
+                'self',
+                'a',
+                'b',
+                'c=42',
+                'd="string"',
+                'e=tuple',
+                'f=dict',
+                'g=None',
+                'h=list',
+                'i=dict',
+                'j=False',
+                '*args',
+                '**kwargs',
+            ]
+        )
+
+    def test_bunk_whitespace(self):
+        """Whitespace in definition throws off argument parsing"""
+        Source = namedtuple('Source', ['source', 'docstring'])
+
+        obj = Source(
+            source=('    def method_foo(self, a, b,\n'
+                    '                   c):\n'
+                    '        call_something()\n'
+                    '        "This is a docstring"\n'
+                    '        return True\n'),
+            docstring='"This is a docstring"',
+        )
+        self.assertEqual(
+            python.PythonPythonMapper._get_arguments(obj),
+            ['self', 'a', 'b', 'c']
+        )

tox.ini

@@ -5,6 +5,8 @@ envlist = py27,py35,lint,docs
 setenv =
     LANG=C
 deps = -r{toxinidir}/requirements.txt
+    pytest
+    mock
 commands =
     py.test {posargs}