doc: fix and improved documentation

* Adapt Sphinx settings according to the newer version used.
* Fix links to the documentation of external libraries.
* Add and include the diagram image for the available transitions for
  the cumin.transports.State class.
* Improve docstrings for a better generated documentation result.
* Remove unnecessary Sphinx helper functions, now correctly handled by
  Sphinx natively.

Change-Id: I9736654c5f7aab9b16b5e29b9018376e50a7f0b3

cumin/color.py

@@ -33,6 +33,11 @@ class Colored(metaclass=ColoredType):
 
     Available methods are dynamically added based on the keys of the :py:const:`ColoredType.COLORS` dictionary.
     For each color a method with the color name is available to color any object with that specific color code.
+
+    Examples::
+
+        Colored.green(object)
+
     """
 
     disabled = False

cumin/grammar.py

@@ -10,6 +10,7 @@
 
 
 INTERNAL_BACKEND_PREFIX = 'cumin.backends'
+""":py:class:`str` with the prefix for built-in backends."""
 
 
 Backend = namedtuple('Backend', ['keyword', 'name', 'cls'])

cumin/query.py

@@ -29,6 +29,7 @@ def __init__(self, config):
 
         :Parameters:
             according to parent :py:meth:`cumin.backends.BaseQueryAggregator.__init__`.
+
         """
         super().__init__(config)
         external = self.config.get('plugins', {}).get('backends', [])

cumin/transports/__init__.py

@@ -188,26 +188,42 @@ class State:
 
     .. attribute:: current
 
-       :py:class:`int`: the current `state`.
+       :py:class:`int`: the current state.
 
-    .. attribute:: pending, scheduled, running, success, failed, timeout
+    .. attribute:: is_pending
 
-        :py:class:`int`: the available valid states, according to :py:attr:`valid_states`.
+       :py:class:`bool`: :py:data:`True` if the current state is `pending`, :py:data:`False` otherwise.
 
-    .. attribute:: is_pending, is_scheduled, is_running, is_success, is_failed, is_timeout
+    .. attribute:: is_scheduled
 
-       :py:class:`bool`: :py:data:`True` if this is the current `state`, :py:data:`False` otherwise.
+       :py:class:`bool`: :py:data:`True` if the current state is `scheduled`, :py:data:`False` otherwise.
+
+    .. attribute:: is_running
+
+       :py:class:`bool`: :py:data:`True` if the current state is `running`, :py:data:`False` otherwise.
+
+    .. attribute:: is_success
+
+       :py:class:`bool`: :py:data:`True` if the current state is `success`, :py:data:`False` otherwise.
+
+    .. attribute:: is_failed
+
+       :py:class:`bool`: :py:data:`True` if the current state is `failed`, :py:data:`False` otherwise.
+
+    .. attribute:: is_timeout
+
+       :py:class:`bool`: :py:data:`True` if the current state is `timeout`, :py:data:`False` otherwise.
 
     """
 
     valid_states = range(6)
-    """:py:class:`list`: valid states indexes."""
+    """:py:class:`list`: valid states integer indexes."""
 
     pending, scheduled, running, success, failed, timeout = valid_states
-    """Valid states."""
+    """Valid state property, one for each :py:data:`cumin.transports.State.valid_states`."""
 
     states_representation = ('pending', 'scheduled', 'running', 'success', 'failed', 'timeout')
-    """:py:func:`tuple`: tuple with the string representations of the valid states."""
+    """:py:func:`tuple`: Tuple with the string representations of the valid states."""
 
     allowed_state_transitions = {
         pending: (scheduled, ),
@@ -217,8 +233,16 @@ class State:
         failed: (),
         timeout: (),
     }
-    """:py:class:`dict`: dictionary with ``{valid state: tuple of valid states}`` mapping of allowed transitions for
-    any valid state."""
+    """:py:class:`dict`: Dictionary with ``{valid state: tuple of valid states}`` mapping of the allowed transitions
+    between all the possile states.
+
+    This is the diagram of the allowed transitions:
+
+    .. image:: ../../examples/transports_state_transitions.png
+       :alt: State class allowed transitions diagram
+    |
+
+    """
 
     def __init__(self, init=None):
         """State constructor. The initial state is set to `pending` it not provided.
@@ -385,11 +409,12 @@ def update(self, new):
     def _cmp(self, other):
         """Comparison operation. Allow to directly compare a state object to another or to an integer.
 
-        Raises ValueError if the comparing object is not an instance of State or an integer.
-
         Arguments:
             other (mixed): the object to compare the current instance to.
 
+        Raises:
+            ValueError: if the comparing object is not an instance of State or an integer.
+
         """
         if isinstance(other, int):
             return self._state - other
@@ -734,8 +759,10 @@ class ProgressBars:
     def __init__(self):
         """Create the progress bars.
 
-        Note that the progress bars themselves are not initalized at object
-        creation. `init()` needs to be called before using the progress bars.
+        Note:
+            the progress bars themselves are not initalized at object creation. ``init()`` needs to be called before
+            using the progress bars.
+
         """
         self.pbar_ok = None
         self.pbar_ko = None

cumin/transports/clustershell.py

@@ -129,7 +129,19 @@ def handler(self, value):
 
 
 class Node:
-    """Node class to represent each target node."""
+    """Node class to represent each target node.
+
+    Additional attributes available are:
+
+    .. attribute:: state
+
+       :py:class:`cumin.transports.State`: the state of the node.
+
+    .. attribute:: running_command_index
+
+       :py:class:`int`: the index of the current running command in the list of commands.
+
+    """
 
     def __init__(self, name, commands):
         """Node class constructor with default values.

doc/source/api/cumin.backends.direct.rst

@@ -2,4 +2,3 @@ Direct
 ======
 
 .. automodule:: cumin.backends.direct
-   :special-members: __init__

doc/source/api/cumin.backends.knownhosts.rst

@@ -2,4 +2,3 @@ KnownHosts
 ==========
 
 .. automodule:: cumin.backends.knownhosts
-   :special-members: __init__

doc/source/api/cumin.backends.openstack.rst

@@ -2,4 +2,3 @@ OpenStack
 =========
 
 .. automodule:: cumin.backends.openstack
-   :special-members: __init__

doc/source/api/cumin.backends.puppetdb.rst

@@ -2,4 +2,3 @@ PuppetDB
 ========
 
 .. automodule:: cumin.backends.puppetdb
-   :special-members: __init__

doc/source/api/cumin.backends.rst

@@ -2,7 +2,6 @@ Backends
 ========
 
 .. automodule:: cumin.backends
-   :special-members: __init__
 
 .. rubric:: Available backends
 

doc/source/api/cumin.color.rst

@@ -0,0 +1,4 @@
+Color
+=====
+
+.. automodule:: cumin.color

doc/source/api/cumin.grammar.rst

@@ -2,5 +2,7 @@ Grammar
 =======
 
 .. automodule:: cumin.grammar
-   :no-private-members:
-   :no-inherited-members:
+   :exclude-members: Backend
+
+   .. autoclass:: Backend
+      :no-private-members:

doc/source/api/cumin.query.rst

@@ -2,4 +2,3 @@ Query
 =====
 
 .. automodule:: cumin.query
-   :special-members: __init__

doc/source/api/cumin.transports.clustershell.rst

@@ -2,4 +2,3 @@ ClusterShell
 ============
 
 .. automodule:: cumin.transports.clustershell
-   :special-members: __init__

doc/source/api/cumin.transports.rst

@@ -2,14 +2,13 @@ Transports
 ==========
 
 .. automodule:: cumin.transports
-   :special-members: __init__
    :exclude-members: Command,State
 
     .. autoclass:: Command
-       :special-members: __init__,__repr__,__str__,__eq__,__ne__
+       :special-members: __repr__,__str__,__eq__,__ne__
 
     .. autoclass:: State
-       :special-members: __init__,__repr__,__str__,__getattr__,__cmp__
+       :special-members: __repr__,__str__,__getattr__,__cmp__
 
 .. rubric:: Available transports
 

doc/source/api/index.rst

@@ -4,17 +4,16 @@ Python API
 Cumin Python API autodoc.
 
 .. automodule:: cumin
-  :no-inherited-members:
-  :special-members: __init__,__new__
 
 .. autodata:: __version__
 
 .. rubric:: Subpackages and Submodules
 
 .. toctree::
 
+   cumin.backends
+   cumin.color
    cumin.grammar
    cumin.query
    cumin.transport
-   cumin.backends
    cumin.transports

doc/source/conf.py

@@ -17,10 +17,8 @@
 # add these directories to sys.path here. If the directory is relative to the
 # documentation root, use os.path.abspath to make it absolute, like shown here.
 #
-import importlib
 import os
 import sys
-import types
 
 import sphinx_rtd_theme
 
@@ -66,7 +64,7 @@
 # General information about the project.
 project = u'Cumin'
 title = u'{project} Documentation'.format(project=project)
-copyright = u"2017-2018, Riccardo Coccioli <rcoccioli@wikimedia.org>, Wikimedia Foundation, Inc."
+copyright = u"2017-2020, Riccardo Coccioli <rcoccioli@wikimedia.org>, Wikimedia Foundation, Inc."
 author = u'Riccardo Coccioli'
 
 # The version info for the project you're documenting, acts as replacement for
@@ -130,8 +128,8 @@
 
 intersphinx_mapping = {
     'python': ('https://docs.python.org/3/', None),
-    'requests': ('//requests.readthedocs.io/en/master/', None),
-    'ClusterShell': ('http://clustershell.readthedocs.io/en/v1.8/', None),
+    'requests': ('https://requests.readthedocs.io/en/master/', None),
+    'ClusterShell': ('https://clustershell.readthedocs.io/en/v1.8.1/', None),
     'keystoneauth1': ('https://docs.openstack.org/keystoneauth/latest', None),
     'novaclient': ('https://docs.openstack.org/python-novaclient/latest/', None),
     'pyparsing': ('https://pythonhosted.org/pyparsing/', 'pyparsing.inv'),
@@ -152,69 +150,37 @@
 napoleon_use_keyword = True
 
 # Autodoc settings
-autodoc_default_flags = ['members', 'show-inheritance', 'inherited-members', 'private-members']
+autodoc_default_options = {
+    # Using None as value instead of True to support the version of Sphinx used in Buster
+    'members': None,
+    'member-order': 'bysource',
+    'private-members': None,
+    'show-inheritance': None,
+}
+autoclass_content = 'both'
 
 
 # -- Helper functions -----------------------------------------------------
-
 def filter_namedtuple_docstrings(app, what, name, obj, options, lines):
     """Fix the automatically generated docstrings for namedtuples classes."""
-    if what == 'attribute' and len(lines) == 1 and lines[0].startswith('Alias for field number'):
-        lines[0] = 'See the Keyword Arguments description.'
-
-
-def add_abstract_annotations(app, what, name, obj, options, lines):
-    """Workaround to add an abstract annotation for ABC abstract classes and abstract methods."""
-    if ((what in ('method', 'attribute') and getattr(obj, '__isabstractmethod__', False))
-            or (what == 'class' and len(getattr(obj, '__abstractmethods__', [])) > 0)):
-        lines.insert(0, '``abstract``')
+    if what == 'property' and len(lines) == 1 and lines[0].startswith('Alias for field number'):
+        del lines[:]
 
 
-def add_inherited_annotations(app, what, name, obj, options, lines):
-    """Workaround to add an inherited annotation for methods inherited from the parent classes."""
-    if what == 'method':
-        if hasattr(obj, 'im_class'):
-            if obj.__name__ not in obj.im_class.__dict__:
-                lines.insert(0, '``inherited``')
-        elif isinstance(obj, types.FunctionType):  # Static methods
-            module_name, class_name, _ = name.rsplit('.', 2)
-            module = importlib.import_module(module_name)  # Dynamically import the module
-            if obj.__name__ not in getattr(module, class_name).__dict__:  # Dynamically inspect the class
-                lines.insert(0, '``inherited``')
-                lines.insert(0, '``static``')
+# Keep track of documented classes to avoid annotating both class and __init__.
+# Necessary when using autoclass_content 'both' and add_abstract_annotations().
+_cumin_documented_classes = set()
 
-    elif what == 'attribute':
-        module_name, class_name, prop = name.rsplit('.', 2)
-        module = importlib.import_module(module_name)  # Dynamically import the module
-        if prop not in getattr(module, class_name).__dict__:  # Dynamically inspect the class
-            lines.insert(0, '``inherited``')
 
-
-def skip_external_inherited(app, what, name, obj, skip, options):
-    """Skip methods inherited from external libraries."""
-    if not (what == 'class' and hasattr(obj, 'im_class') and obj.__name__ not in obj.im_class.__dict__):
-        return
-
-    classes = [obj.im_class]
-    while classes:
-        c = classes.pop()
-        if obj.__name__ in c.__dict__:  # Found the class that defined the method
-            return 'cumin' not in c.__module__  # Skip if from external libraries
-        else:
-            classes = list(c.__bases__) + classes  # Continue in the inheritance tree
-
-
-def skip_exceptions_init(app, what, name, obj, skip, options):
-    """Skip __init__ method for Exception classes."""
-    if what == 'exception' and name == '__init__':
-        return True
+def add_abstract_annotations(app, what, name, obj, options, lines):
+    """Workaround to add an abstract annotation for ABC abstract classes."""
+    if what == 'class' and len(getattr(obj, '__abstractmethods__', [])) > 0 and name not in _cumin_documented_classes:
+        lines.insert(0, '``abstract``')
+        _cumin_documented_classes.add(name)
 
 
 def setup(app):
-    """Register the filter_namedtuple_docstrings function."""
+    """Register the helper functions."""
     app.connect('autodoc-process-docstring', filter_namedtuple_docstrings)
     app.connect('autodoc-process-docstring', add_abstract_annotations)
-    app.connect('autodoc-process-docstring', add_inherited_annotations)
-    app.connect('autodoc-skip-member', skip_exceptions_init)
-    app.connect('autodoc-skip-member', skip_external_inherited)
     app.add_stylesheet('theme_overrides.css')  # override wide tables in RTD theme

doc/source/transports_state_transitions.dot

@@ -0,0 +1,11 @@
+digraph G {
+    rankdir=TB;
+
+    pending -> scheduled;
+    scheduled -> running;
+    running -> running;
+    running -> success;
+    running -> failed;
+    running -> timeout;
+    success -> pending;
+}