Added test to do some basic checking of docstrings

Author: moagstar

star_alchemy/_star_alchemy.py

@@ -31,6 +31,9 @@ class Join:
 
     @property
     def name(self) -> str:
+        """
+        :return: The name of the right hand table in this join.
+        """
         assert self.table.name is not None
         return self.table.name
 
@@ -63,35 +66,52 @@ class StarSchema:
     @property
     def tables(self) -> SqlAlchemyTableDict:
         """
-        :return:
+        :return: Dictionary mapping names to the SQLAlchemy tables
+                 referenced by this schema.
         """
         if self._tables is None:
             self._tables = {s.name: s.table for s in self}
         return self._tables
 
     def select(self, *args, **kwargs) -> StarSchemaSelect:
         """
+        :param args: star args to pass to select
+        :param kwargs: double star args to pass to select
+
+        For more information consult the SQLAlchemy documentation:
 
-        :return:
+        https://docs.sqlalchemy.org/en/latest/core/selectable.html#sqlalchemy.sql.expression.select
+
+        :return: StarSchemaSelect instance which will auto generate it's
+                 select_from upon compilation.
         """
         return StarSchemaSelect(self, *args, **kwargs)
 
     @property
     def schemas(self) -> StarSchemaDict:
         """
-        :return:
+        :return: Dictionary mapping table names to subschemas.
         """
         if self._schemas is None:
             self._schemas = {s.name: s for s in self}
         return self._schemas
 
-    def __getitem__(self, item) -> 'StarSchema':
-        return self.schemas[item]
+    def __getitem__(self, name) -> 'StarSchema':
+        """
+        Traverse the tree and retrieve the sub schema with the given
+        name.
+
+        :param name: The name (this is the name of the SQLAlchemy table
+                     the sub schema holds) of the sub schema to get.
+
+        :return: Sub schema referenced by the requested name.
+        """
+        return self.schemas[name]
 
     @property
     def path(self) -> typing.List['StarSchema']:
         """
-        :return:
+        :return: Path to the root of the schema.
         """
         def make_path(star_schema) -> typing.Iterator[StarSchema]:
             yield star_schema
@@ -100,10 +120,16 @@ def make_path(star_schema) -> typing.Iterator[StarSchema]:
 
     @property
     def name(self) -> str:
+        """
+        :return: The name of the table this schema references.
+        """
         return self.join.name
 
     @property
     def table(self) -> SqlAlchemyTable:
+        """
+        :return: The Sqlachemy table referenced by this schema.
+        """
         return self.join.table
 
     def detach(self, table_name: str) -> 'StarSchema':
@@ -130,8 +156,8 @@ def clone(schema, parent):
 
     def __iter__(self) -> typing.Iterator['StarSchema']:
         """
-
-        :return:
+        :return: Iterator which traverses the tree in a depth first
+                 fashion.
         """
         def recurse(star_schema: StarSchema) -> typing.Iterable['StarSchema']:
             yield star_schema
@@ -140,6 +166,9 @@ def recurse(star_schema: StarSchema) -> typing.Iterable['StarSchema']:
         return iter(recurse(self))
 
     def __hash__(self) -> int:
+        """
+        :return: Hash value to uniquely identify this instance.
+        """
         return hash(self.join) | hash(self.parent)
 
     @classmethod
@@ -195,14 +224,10 @@ def compile_star_schema_select(element: StarSchemaSelect, compiler, **kw):
     # edges in these paths are the joins that need to be made
     joins = (
         star_schema
-
         for expression in iterate(element, {'column_collections': False})
         if isinstance(expression, Column)                       # only interested in columns
         if not isinstance(expression.table, StarSchemaSelect)   # but not the select itself
-
-        for star_schema in element.star_schema[expression.table.name].path
-        if star_schema.parent is not None                       # don't need to create a join from
-                                                                # root -> root
+        for star_schema in element.star_schema[expression.table.name].path[1:]
     )
 
     # generate the select_from using all the referenced tables

tests/test_star_schema.py

@@ -1,10 +1,14 @@
+import inspect
+import re
+import typing
+from contextlib import suppress
 from unittest import TestCase
 
 import sqlalchemy as sa
 
 from examples.sales import tables
 from star_alchemy import _star_alchemy
-from star_alchemy._star_alchemy import Join, StarSchema
+from star_alchemy._star_alchemy import Join, StarSchema, StarSchemaSelect
 from tests import tables
 from tests.util import AssertQueryEqualMixin, DocTestMixin, query_test
 
@@ -196,5 +200,71 @@ def test_detach(self):
         return product.select([product.tables['category'].c.id])
 
 
-class DocTestCase(TestCase, DocTestMixin(_star_alchemy)):
-    pass
+class DocStringTestCase(TestCase, DocTestMixin(_star_alchemy)):
+    """
+    Check the docstrings of star_alchemy module, couldn't find a library
+    that checks docstrings in rst format so fudging my own. The main
+    things I care about...
+
+    * Parameters and return types are correctly documented
+    * Examples are correct.
+
+    Most docstring checking focus on the format, which is less important
+    for me.
+    """
+
+    @property
+    def sub_tests(self):
+        """
+        Iterate through all the functions which should have their
+        docstring checked for completeness.
+        """
+        def filter_member(o):
+            return (
+                (inspect.isfunction(o) or isinstance(o, property))
+                and (o.__doc__ is None or not o.__doc__.startswith('Method generated by attrs'))
+            )
+
+        # TODO: StarSchemaSelect, compile_star_schema_select
+        for cls in StarSchema, Join:
+            for name, member in inspect.getmembers(cls, predicate=filter_member):
+                if name in ("__delattr__", "__setattr__"):
+                    continue
+
+                if isinstance(member, property):
+                    file = inspect.getfile(member.fget)
+                    line = inspect.getsourcelines(member.fget)[1]
+                else:
+                    file = inspect.getfile(member)
+                    line = inspect.getsourcelines(member)[1]
+                yield f'{cls.__name__}.{name}', member, f'File "{file}", line {line}'
+
+    def test_docstring_should_be_present(self):
+        for name, member, location in self.sub_tests:
+            with self.subTest(name):
+                if member.__doc__ is None:
+                    self.fail(f'Missing docstring ({location})')
+
+    def test_docstring_return_should_be_documented(self):
+        for name, member, location in self.sub_tests:
+            with self.subTest(name):
+                if member.__doc__ is None:
+                    self.skipTest(f'Missing docstring ({location})')
+                if ":return:" not in member.__doc__:
+                    self.fail(f'Missing :return: ({location})')
+                if re.search(r':return:\s*\n', member.__doc__):
+                    self.fail(f'Empty :return: ({location})')
+
+    def test_all_parameters_should_be_documented(self):
+        for name, member, location in self.sub_tests:
+            with self.subTest(name):
+                if not isinstance(member, property):
+                    if member.__doc__ is None:
+                        self.skipTest(f'Missing docstring ({location})')
+                    for parameter in inspect.signature(member).parameters.values():
+                        param = f':param {parameter.name}:'
+                        if parameter.name != 'self':
+                            if param not in member.__doc__:
+                                self.fail(f'Missing {param} ({location})')
+                        if re.search(rf'{param}\s*\n', member.__doc__):
+                            self.fail(f'Empty {param} ({location})')