Fixing documentation, WIP for enums

modules/python/doc/CMakeLists.txt

@@ -70,6 +70,7 @@ add_custom_target(visp_python_bindings_doc
 		-b html
 		-c "${BINARY_BUILD_DIR}"
 		-d "${SPHINX_CACHE_DIR}"
+    -j 8
 		"${CMAKE_CURRENT_SOURCE_DIR}"
 		"${SPHINX_HTML_DIR}"
 	COMMENT "Building Sphinx HTML documentation for ViSP's Python bindings"

modules/python/doc/_templates/custom-class-template.rst

@@ -16,7 +16,7 @@
    .. autosummary::
       :nosignatures:
    {% for item in methods %}
-      {%- if not item.startswith('_') and item not in inherited_members %}
+      {%- if not item.startswith('_') and item not in inherited_members or item.startswith('__init__') %}
       ~{{ name }}.{{ item }}
       {%- endif -%}
    {%- endfor %}

modules/python/doc/conf.py.in

@@ -31,7 +31,6 @@ sys.path.insert(0, os.path.abspath('../build'))
 extensions = [
     "sphinx.ext.autodoc",
     "sphinx.ext.intersphinx",
-    "sphinx.ext.napoleon",
     "sphinx.ext.mathjax",
     "sphinx.ext.autosummary",
     "sphinx_immaterial",
@@ -384,11 +383,31 @@ object_description_options = [
   ("py:.*", dict(include_fields_in_toc=False)),
 ]
 
+python_type_aliases = {
+  "_visp.": "visp.",
+}
+
+
+autodoc_excludes = [
+'__weakref__',  '__doc__', '__module__', '__dict__',
+'__dir__', '__delattr__', '__format__', '__init_subclass__', '__new__',
+'__reduce__', '__reduce_ex__', '__repr__', '__setattr__',
+'__sizeof__', '__str__', '__subclasshook__', '__getattribute__'
+]
+def autodoc_skip_member(app, what, name, obj, skip, options):
+  # Ref: https://stackoverflow.com/a/21449475/
+
+  exclude = name in autodoc_excludes
+  # return True if (skip or exclude) else None  # Can interfere with subsequent skip functions.
+  return True if exclude else skip
 
 def setup(app):
   import logging
   from sphinx.util.logging import NAMESPACE
   logger = logging.getLogger(NAMESPACE)
   for handler in logger.handlers:
-    handler.addFilter(FilterPybindArgWarnings(app))
-    handler.addFilter(FilterNoIndexWarnings(app))
+    if isinstance(handler, WarningStreamHandler):
+      handler.addFilter(FilterPybindArgWarnings(app))
+      handler.addFilter(FilterNoIndexWarnings(app))
+
+  app.connect('autodoc-skip-member', autodoc_skip_member)

modules/python/generator/visp_python_bindgen/doc_parser.py

@@ -114,10 +114,16 @@ class MethodDocumentation(object):
 @dataclass
 class ClassDocumentation(object):
   documentation: str
+@dataclass
+class EnumDocumentation(object):
+  general_documentation: str
+  value_documentation: Dict[str, str]
+
 
 @dataclass
 class DocElements(object):
   compounddefs: Dict[str, compounddefType]
+  enums: Dict[str, doxmlparser.memberdefType]
   methods: Dict[Tuple[str, MethodDocSignature], List[doxmlparser.memberdefType]]
 
 IGNORED_MIXED_CONTAINERS = [
@@ -130,6 +136,9 @@ class DocElements(object):
   'date',
 ]
 
+def escape_for_rst(text: str) -> str:
+  return text
+
 def process_mixed_container(container: MixedContainer, level: int, level_string='') -> str:
   '''
   :param level_string: the string being built for a single level (e.g. a line/paragraph of text)
@@ -141,15 +150,15 @@ def process_mixed_container(container: MixedContainer, level: int, level_string=
   requires_space = not level_string.endswith(('\n', '\t', ' ')) and len(level_string) > 0
   # Inline blocks
   if isinstance(container.value, str):
-    return level_string + container.value.replace('\n', '\n' + indent_str).strip()
+    return level_string + escape_for_rst(container.value.replace('\n', '\n' + indent_str).strip())
   if container.name == 'text':
-    return container.value.replace('\n', '\n' + indent_str).strip()
+    return escape_for_rst(container.value.replace('\n', '\n' + indent_str).strip())
   if container.name == 'bold':
     markup_start = '**' if not requires_space or len(level_string) == 0 else ' **'
     return level_string + markup_start + container.value.valueOf_ + '** '
   if container.name == 'computeroutput':
     markup_start = '`' if not requires_space or len(level_string) == 0 else ' `'
-    return level_string + markup_start + container.value.valueOf_ + '` '
+    return level_string + markup_start + escape_for_rst(container.value.valueOf_) + '` '
   if container.name == 'emphasis':
     markup_start = '*' if not requires_space else ' *'
     return level_string + markup_start + container.value.valueOf_ + '* '
@@ -266,6 +275,7 @@ def __init__(self, path: Optional[Path], env_mapping: Dict[str, str]):
       else:
         self.xml_doc = doxmlparser.compound.parse(str(path), True, False)
         compounddefs_res = {}
+        enums_res = {}
         methods_res = {}
         for compounddef in self.xml_doc.get_compounddef():
           compounddef: compounddefType = compounddef
@@ -275,6 +285,7 @@ def __init__(self, path: Optional[Path], env_mapping: Dict[str, str]):
             section_defs: List[doxmlparser.sectiondefType] = compounddef.sectiondef
             for section_def in section_defs:
               member_defs: List[doxmlparser.memberdefType] = section_def.memberdef
+              enum_defs = [d for d in member_defs if d.kind == doxmlparser.compound.DoxMemberKind.ENUM and d.prot == 'public']
               method_defs = [d for d in member_defs if d.kind == doxmlparser.compound.DoxMemberKind.FUNCTION and d.prot == 'public']
               for method_def in method_defs:
                 is_const = False if method_def.const == 'no' else True
@@ -283,7 +294,6 @@ def __init__(self, path: Optional[Path], env_mapping: Dict[str, str]):
                 param_types = []
                 for param in method_def.get_param():
                   t = ''.join(process_mixed_container(c, 0) for c in param.type_.content_)
-
                   param_types.append(t)
                 if method_def.name == cls_name or ret_type != '':
                   signature_str = f'{ret_type} {cls_name}::{method_def.name}({",".join(param_types)}) {{}}'
@@ -302,7 +312,11 @@ def __init__(self, path: Optional[Path], env_mapping: Dict[str, str]):
                       methods_res[key] = method_def
                   else:
                     methods_res[key] = method_def
-        self.elements = DocElements(compounddefs_res, methods_res)
+
+              for enum_def in enum_defs:
+                enums_res[compounddef.get_compoundname() + '::' +  enum_def.name] = enum_def
+
+        self.elements = DocElements(compounddefs_res, enums_res, methods_res)
 
   def get_documentation_for_class(self, name: str, cpp_ref_to_python: Dict[str, str], specs: Dict[str, str]) -> Optional[ClassDocumentation]:
     compounddef = self.elements.compounddefs.get(name)
@@ -311,6 +325,20 @@ def get_documentation_for_class(self, name: str, cpp_ref_to_python: Dict[str, st
     cls_str = to_cstring(self.generate_class_description_string(compounddef))
     return ClassDocumentation(cls_str)
 
+  def get_documentation_for_enum(self, enum_name: str) -> Optional[EnumDocumentation]:
+    member_def = self.elements.enums.get(enum_name)
+    print(self.elements.enums)
+    if member_def is None:
+      return None
+    general_doc = to_cstring(self.generate_method_description_string(member_def))
+    value_doc = {}
+    for enum_val in member_def.enumvalue:
+      enum_value: doxmlparser.enumvalueType = enum_val
+      brief = process_description(enum_value.briefdescription)
+      detailed = process_description(enum_value.detaileddescription)
+      value_doc[enum_value.name] = to_cstring(brief + '\n\n' + detailed)
+    return EnumDocumentation(general_doc, value_doc)
+
   def generate_class_description_string(self, compounddef: compounddefType) -> str:
     brief = process_description(compounddef.get_briefdescription())
     detailed = process_description(compounddef.get_detaileddescription())

modules/python/generator/visp_python_bindgen/enum_binding.py

@@ -46,7 +46,7 @@
 
 from typing import TYPE_CHECKING
 if TYPE_CHECKING:
-  from visp_python_bindgen.header import SingleObjectBindings
+  from visp_python_bindgen.header import SingleObjectBindings, HeaderFile
 
 @dataclass
 class EnumRepr:
@@ -173,7 +173,7 @@ def accumulate_data(scope: Union[NamespaceScope, ClassScope]):
   accumulate_data(root_scope)
   return final_data, temp_data
 
-def get_enum_bindings(root_scope: NamespaceScope, mapping: Dict, submodule: Submodule) -> List[SingleObjectBindings]:
+def get_enum_bindings(root_scope: NamespaceScope, mapping: Dict, submodule: Submodule, header: 'HeaderFile') -> List[SingleObjectBindings]:
 
   final_data, filtered_reprs = resolve_enums_and_typedefs(root_scope, mapping)
 
@@ -183,6 +183,7 @@ def get_enum_bindings(root_scope: NamespaceScope, mapping: Dict, submodule: Subm
   result: List['SingleObjectBindings'] = []
   final_reprs = []
   for repr in final_data:
+
     enum_config = submodule.get_enum_config(repr.name)
     if enum_config['ignore']:
       filtered_reprs.append(repr)
@@ -191,13 +192,16 @@ def get_enum_bindings(root_scope: NamespaceScope, mapping: Dict, submodule: Subm
       final_reprs.append(repr)
     else:
       filtered_reprs.append(repr)
-
+  doc_holder = header.documentation_holder
   for enum_repr in final_reprs:
     name_segments = enum_repr.name.split('::')
     py_name = name_segments[-1].replace('vp', '')
     # If an owner class is ignored, don't export this enum
     parent_ignored = False
     ignored_parent_name = None
+    enum_doc = None
+    if doc_holder is not None:
+      enum_doc = header.documentation_holder.get_documentation_for_enum(repr.name)
 
     for segment in name_segments[:-1]:
       full_segment_name = mapping.get(segment)
@@ -213,11 +217,21 @@ def get_enum_bindings(root_scope: NamespaceScope, mapping: Dict, submodule: Subm
     owner_full_name = '::'.join(name_segments[:-1])
     owner_py_ident = get_owner_py_ident(owner_full_name, root_scope) or 'submodule'
     py_ident = f'py{owner_py_ident}{py_name}'
+    py_args = ['py::arithmetic()']
+    if enum_doc is not None:
+      if enum_doc.general_documentation is not None:
+        py_args = [enum_doc.general_documentation] + py_args
 
-    declaration = f'py::enum_<{enum_repr.name}> {py_ident}({owner_py_ident}, "{py_name}", py::arithmetic());'
+    py_args_str = ','.join(py_args)
+    declaration = f'py::enum_<{enum_repr.name}> {py_ident}({owner_py_ident}, "{py_name}", {py_args_str});'
     values = []
     for enumerator in enum_repr.values:
-      values.append(f'{py_ident}.value("{enumerator.name}", {enum_repr.name}::{enumerator.name});')
+      maybe_value_doc = None
+      if enum_doc is not None:
+        maybe_value_doc = enum_doc.value_documentation.get(enumerator.name)
+      maybe_value_doc_str = f', {maybe_value_doc}' if maybe_value_doc else ''
+
+      values.append(f'{py_ident}.value("{enumerator.name}", {enum_repr.name}::{enumerator.name}{maybe_value_doc_str});')
 
     values.append(f'{py_ident}.export_values();')
     enum_names = BoundObjectNames(py_ident, py_name, enum_repr.name, enum_repr.name)

modules/python/generator/visp_python_bindgen/header.py

@@ -184,7 +184,6 @@ def parse_data(self, bindings_container: BindingsContainer) -> None:
     '''
     Update the bindings container passed in parameter with the bindings linked to this header file
     '''
-
     from visp_python_bindgen.enum_binding import get_enum_bindings
     # Fetch documentation if available
     if self.documentation_holder_path is not None:
@@ -194,14 +193,19 @@ def parse_data(self, bindings_container: BindingsContainer) -> None:
 
     for cls in self.header_repr.namespace.classes:
       self.generate_class(bindings_container, cls, self.environment)
-    enum_bindings = get_enum_bindings(self.header_repr.namespace, self.environment.mapping, self.submodule)
+    enum_bindings = get_enum_bindings(self.header_repr.namespace, self.environment.mapping, self.submodule, self)
     for enum_binding in enum_bindings:
       bindings_container.add_bindings(enum_binding)
 
     # Parse functions that are not linked to a class
     self.parse_sub_namespace(bindings_container, self.header_repr.namespace)
 
   def parse_sub_namespace(self, bindings_container: BindingsContainer, ns: NamespaceScope, namespace_prefix = '', is_root=True) -> None:
+    '''
+    Parse a subnamespace and all its subnamespaces.
+    In a namespace, only the functions are exported.
+    '''
+
     if not is_root and ns.name == '': # Anonymous namespace, only visible in header, so we ignore it
       return
 
@@ -228,20 +232,59 @@ def parse_sub_namespace(self, bindings_container: BindingsContainer, ns: Namespa
       self.parse_sub_namespace(bindings_container, ns.namespaces[sub_ns], namespace_prefix + sub_ns + '::', False)
 
   def generate_class(self, bindings_container: BindingsContainer, cls: ClassScope, header_env: HeaderEnvironment) -> SingleObjectBindings:
+    '''
+    Generate the bindings for a single class:
+    This method will generate one Python class per template instanciation.
+    If the class has no template argument, then a single python class is generated
+
+    If it is templated, the mapping (template argument types => Python class name) must be provided in the JSON config file
+    '''
     def generate_class_with_potiental_specialization(name_python: str, owner_specs: OrderedDict[str, str], cls_config: Dict) -> str:
+      '''
+      Generate the bindings of a single class, handling a potential template specialization.
+      The handled information is:
+        - The inheritance of this class
+        - Its public fields that are not pointers
+        - Its constructors
+        - Most of its operators
+        - Its public methods
+      '''
       python_ident = f'py{name_python}'
       name_cpp = get_typename(cls.class_decl.typename, owner_specs, header_env.mapping)
       class_doc = None
+
       methods_dict: Dict[str, List[MethodBinding]] = {}
-      def add_to_method_dict(key, value):
+      def add_to_method_dict(key: str, value: MethodBinding):
+        '''
+        Add a method binding to the dictionary containing all the methods bindings of the class.
+        This dict is a mapping str => List[MethodBinding]
+        '''
         if key not in methods_dict:
           methods_dict[key] = [value]
         else:
           methods_dict[key].append(value)
+
+      def add_method_doc_to_pyargs(method: types.Method, py_arg_strs: List[str]) -> List[str]:
+        if self.documentation_holder is not None:
+          method_name = get_name(method.name)
+          method_doc_signature = MethodDocSignature(method_name,
+                                                    get_type(method.return_type, {}, header_env.mapping) or '', # Don't use specializations so that we can match with doc
+                                                    [get_type(param.type, {}, header_env.mapping) for param in method.parameters],
+                                                    method.const, method.static)
+          method_doc = self.documentation_holder.get_documentation_for_method(name_cpp_no_template, method_doc_signature, {}, owner_specs, param_names, [])
+          if method_doc is None:
+            logging.warning(f'Could not find documentation for {name_cpp}::{method_name}!')
+            return py_arg_strs
+          else:
+            return [method_doc.documentation] + py_arg_strs
+        else:
+          return py_arg_strs
+
       if self.documentation_holder is not None:
         class_doc = self.documentation_holder.get_documentation_for_class(name_cpp_no_template, {}, owner_specs)
       else:
         logging.warning(f'Documentation not found when looking up {name_cpp_no_template}')
+
       # Declaration
       # Add template specializations to cpp class name. e.g., vpArray2D becomes vpArray2D<double> if the template T is double
       template_decl: Optional[types.TemplateDecl] = cls.class_decl.template
@@ -302,16 +345,8 @@ def add_to_method_dict(key, value):
           params_strs = [get_type(param.type, owner_specs, header_env.mapping) for param in method.parameters]
           py_arg_strs = get_py_args(method.parameters, owner_specs, header_env.mapping)
           param_names = [param.name or 'arg' + str(i) for i, param in enumerate(method.parameters)]
-          if self.documentation_holder is not None:
-            method_doc_signature = MethodDocSignature(method_name,
-                                                      get_type(method.return_type, {}, header_env.mapping) or '', # Don't use specializations so that we can match with doc
-                                                      [get_type(param.type, {}, header_env.mapping) for param in method.parameters],
-                                                      method.const, method.static)
-            method_doc = self.documentation_holder.get_documentation_for_method(name_cpp_no_template, method_doc_signature, {}, owner_specs, param_names, [])
-            if method_doc is None:
-              logging.warning(f'Could not find documentation for {name_cpp}::{method_name}!')
-            else:
-              py_arg_strs = [method_doc.documentation] + py_arg_strs
+
+          py_arg_strs = add_method_doc_to_pyargs(method, py_arg_strs)
 
           ctor_str = f'''{python_ident}.{define_constructor(params_strs, py_arg_strs)};'''
           add_to_method_dict('__init__', MethodBinding(ctor_str, is_static=False, is_lambda=False,
@@ -329,6 +364,10 @@ def add_to_method_dict(key, value):
         return_type_str = get_type(method.return_type, owner_specs, header_env.mapping)
         py_args = get_py_args(method.parameters, owner_specs, header_env.mapping)
         py_args = py_args + ['py::is_operator()']
+        param_names = [param.name or 'arg' + str(i) for i, param in enumerate(method.parameters)]
+
+        py_args = add_method_doc_to_pyargs(method, py_args)
+
         if len(params_strs) > 1:
           logging.info(f'Found operator {name_cpp}{method_name} with more than one parameter, skipping')
           rejection = RejectedMethod(name_cpp, method, method_config, get_method_signature(method_name, return_type_str, params_strs), NotGeneratedReason.NotHandled)
@@ -381,15 +420,17 @@ def add_to_method_dict(key, value):
             method_str, method_data = define_method(method, method_config, True,
                                                                 new_specs, self, header_env, class_def_names)
             add_to_method_dict(method_data.py_name, MethodBinding(method_str, is_static=method.static,
-                                                                        is_lambda=f'{name_cpp}::*' not in method_str,
-                                                                        is_operator=False, is_constructor=False, method_data=method_data))
+                                                                  is_lambda=f'{name_cpp}::*' not in method_str,
+                                                                  is_operator=False, is_constructor=False,
+                                                                  method_data=method_data))
             generated_methods.append(method_data)
         else:
           method_str, method_data = define_method(method, method_config, True,
                                                               owner_specs, self, header_env, class_def_names)
           add_to_method_dict(method_data.py_name, MethodBinding(method_str, is_static=method.static,
-                                                                      is_lambda=f'{name_cpp}::*' not in method_str,
-                                                                      is_operator=False, is_constructor=False, method_data=method_data))
+                                                                is_lambda=f'{name_cpp}::*' not in method_str,
+                                                                is_operator=False, is_constructor=False,
+                                                                method_data=method_data))
           generated_methods.append(method_data)
 
       # See https://github.com/pybind/pybind11/issues/974