tools/generate-settings-dataclasses.py: Generate attr docstrings(1/2)

Prepare to generate docstrings for the attributes of settings_classes.
Even though they are not parsed by python itself, they can be
used to generate documentation for the settings dataclasses.

To not require manual edits after code generation, it was needed to
implement sorting of required attributes like connection_id, -type
and uuid before other attributes. The new functions for this are
derived from the existing sort function for the settings_classes
where we need the connection as first attribute of the profile
because likewise, it is not an optional attribute.

The switch to enable generating the attribute docstrings is deferred
to the next commit, to make reviewing this change above (which does
not change the established code) and the minor change below easier:

There is a minor change in the established code but only regarding
sdbus_async/networkmanager/settings/bond.py whose attributes
are not actually required and should be optional. Making them
required was a quick hack around some issue I don't recall the
exact details of, and should be adressed at its source anyway.

This is the only code change as result of running the updated
tool, and the next commit will enable the switch to generate
the docstrings.

To aid in understanding and (in the future, reworking) the
code for use in the new upcoming generator based on Jinja2,
this also adds a few comments and docstrings for navigating
the code.

I guess this may help be a step towards adressing PR #31,
for documenting new Settings helpers classes.

Author: Bernhard Kaindl

sdbus_async/networkmanager/settings/bond.py

@@ -11,9 +11,11 @@
 class BondSettings(NetworkManagerSettingsMixin):
     """Bonding Settings"""
 
-    interface_name: str = field(
+    interface_name: Optional[str] = field(
         metadata={'dbus_name': 'interface-name', 'dbus_type': 's'},
+        default=None,
     )
-    options: Dict[str, str] = field(
+    options: Optional[Dict[str, str]] = field(
         metadata={'dbus_name': 'options', 'dbus_type': 'a{ss}'},
+        default={'Mode': 'Balance-Rr'},
     )

tools/generate-settings-dataclasses.py

@@ -1,4 +1,5 @@
 #!/usr/bin/env python
+# Based on NetworkManager-1.39.2/tools/generate-docs-nm-settings-docs-merge.py
 # SPDX-License-Identifier: LGPL-2.1-or-later
 import collections
 import textwrap
@@ -57,6 +58,7 @@ def write(msg: Any) -> None:
 
 ###############################################################################
 
+# Order of the connection types(settings_classes) in the profile dataclass:
 _setting_name_order = [
     "connection",
     "ipv4",
@@ -112,6 +114,7 @@ def write(msg: Any) -> None:
     "wpan",
 ]
 
+# List of modules for which we must generate an import of typing.List
 list_modules = [
     "bridge",
     "bridge_port",
@@ -134,22 +137,50 @@ def write(msg: Any) -> None:
     "wireless_security",
 ]
 
+# Order of special properties which must be first because they are not optional
+_property_name_order = [
+    "connection.id",
+    "connection.type",
+    "connection.uuid",
+]
+
+def _property_name_order_idx(name: str) -> int:
+    """Return the sort index for the given connection setting property"""
+    try:
+        return _property_name_order.index(name)
+    except ValueError:
+        # Properties not in _property_name_order are sorted last and then by their name
+        return len(_property_name_order)
+
+
+def key_fcn_property_name(n1: str) -> Tuple[int, str]:
+    """key function for sorted(), used to sort connection properties"""
+    return (_property_name_order_idx(n1), n1)
+
 
 def _setting_name_order_idx(name: str) -> int:
+    """Return the sort index for the given connection type(settings_class)"""
     try:
         return _setting_name_order.index(name)
     except ValueError:
         return len(_setting_name_order)
 
 
 def key_fcn_setting_name(n1: str) -> Tuple[int, str]:
+    """key function for sorted(), used to sort the settings_classes(connection types)"""
     return (_setting_name_order_idx(n1), n1)
 
 
 def iter_keys_of_dicts(
-    dicts: List[Dict[str, Any]], key: Optional[Any] = None
+    dicts: List[Dict[str, Any]], key: Optional[Any] = None, prefix: Optional[str] = ""
 ) -> List[str]:
-    keys = {k for d in dicts for k in d.keys()}
+    """Return a sorted list of settings_classes or connection properties
+
+    To support sorting the required properites of settings_classes first,
+    the settingsname can be passed ad prefix argument to let the key function
+    return the correct sort index for the given settingsname property.
+    """
+    keys = {f'{prefix}{k}' for d in dicts for k in d.keys()}
     return sorted(keys, key=key)
 
 
@@ -185,6 +216,8 @@ def find_first_not_none(itr: List[Any]) -> Optional[Any]:
     return next((i for i in itr if i is not None), None)
 
 
+# The code quality of this function is poor(Sourcery says 5%), needs refactoring,
+# also see the rework in tools/generate-settings-dataclasses-jinja.py
 def main(settings_xml_path: Path) -> None:
     gl_input_files = [settings_xml_path]
 
@@ -194,25 +227,30 @@ def main(settings_xml_path: Path) -> None:
                       for root in xml_roots]
 
     root_node = ElementTree.Element("nm-setting-docs")
-    # print("")
+
+    # Generate the file header
     license = "SPDX-License-Identifier: LGPL-2.1-or-later"
     script = ("This file was generated by "
               "tools/generate-settings-dataclasses.py")
     header = f"""# {license}\n# {script},
-    # if possible, please make changes by also updating the script.
-    """
+# if possible, please make changes by also updating the script.\n"""
     i = open("sdbus_async/networkmanager/settings/__init__.py", mode="w")
     p = open("sdbus_async/networkmanager/settings/profile.py", mode="r")
     profile_py = open("sdbus_async/networkmanager/settings/profile.py").read()
+
+    # define start and end markers for generating part of settings/profile.py:
     start_string = "# start of the generated list of settings classes\n"
     start_index = profile_py.index(start_string) + len(start_string)
-    # end_string = "    def to_dbus"
     end_string = "    # end of the generated list of settings classes\n"
     end_index = profile_py.index(end_string)
     p = open("sdbus_async/networkmanager/settings/profile.py", mode="w")
+
+    # write the file headers
     i.write(header)
     p.write(profile_py[:start_index])
     classes = []
+
+    # generate the connection type settings classes:
     for settingname in iter_keys_of_dicts(settings_roots,
                                           key_fcn_setting_name):
         settings = [d.get(settingname) for d in settings_roots]
@@ -235,6 +273,8 @@ def main(settings_xml_path: Path) -> None:
         i.write(f"from .{module} import {classname}\n")
         classes.append(classname)
         f = open(f"sdbus_async/networkmanager/settings/{module}.py", mode="w")
+
+        # Generate module type headers with the needed import statements
         f.write(header)
         f.write("from __future__ import annotations\n")
         f.write("from dataclasses import dataclass, field\n")
@@ -255,6 +295,7 @@ def main(settings_xml_path: Path) -> None:
             f.write("from .datatypes import WireguardPeers as Peers\n")
         f.write("\n\n")
 
+        # Generate the settings_class and it's entry in profile.py:
         setting_node = ElementTree.SubElement(root_node, "setting")
         if module != "connection":
             p.write(f"    {module}: Optional[{classname}] = field(\n")
@@ -265,10 +306,15 @@ def main(settings_xml_path: Path) -> None:
         f.write("@dataclass\n")
         f.write(f"class {classname}(NetworkManagerSettingsMixin):\n")
         setting_node.set("name", settingname)
+
+        # generate the docstring of the new settings_class
         desc = node_get_attr(settings, "description")
         f.write('    """' + desc + '"""\n\n')
-        # node_set_attr(setting_node, "alias", settings)
-        for property_name in iter_keys_of_dicts(properties):
+
+        # Generate the attributes of the settings_class for this profile type:
+
+        for property in iter_keys_of_dicts(properties, key_fcn_property_name, f'{settingname}.'):
+            property_name = property[len(settingname)+1:]
             properties_attrs = [p.get(property_name) for p in properties]
             property_node = ElementTree.SubElement(setting_node, "property")
             property_node.set("name", property_name)
@@ -306,7 +352,7 @@ def main(settings_xml_path: Path) -> None:
                 f.write(f"        default={str(default).title()},\n    )\n")
             else:
                 attribute_type = dbus_type_name_map[dbustype]
-                optional = module != "bond"
+                optional = property not in _property_name_order
                 if optional:
                     f.write(f"    {attribute}: Optional[{attribute_type}]")
                 else:
@@ -329,21 +375,26 @@ def main(settings_xml_path: Path) -> None:
                 if optional:
                     f.write(f"        default={str(default).title()},\n")
                 f.write("    )\n")
+
+                # Generate docstrings for attributes: Not stored by python,
+                # but is parsed for generating documentation and be red by
+                # developers when they lookup the attribute declaration:
                 generate_descriptions_for_attributes = False
                 if generate_descriptions_for_attributes:
                     desc = node_get_attr(properties_attrs, "description")
                     wrapper = textwrap.TextWrapper(
-                        width=74,
+                        width=82,
                         initial_indent="    ",
                         subsequent_indent="    ",
                     )
-                    lines = wrapper.wrap(text=f'"""{desc}')
+                    lines = wrapper.wrap(text=f'"""{desc}"""')
                     if len(lines) == 1:
                         print(lines[0] + '"""')
                     else:
                         for line in lines:
-                            f.write(line)
-                        f.write('    """')
+                            f.write(f'{line}\n')
+                        # If the closing """ shall be on a new line, use:
+                        # f.write('    """\n')
                     f.write("")
     i.write('\n__all__ = (\n')
     for cls in classes: