refactor(ParDict): added more methods to the class

Use the new methods in the code

Author: amilcarlucas

ardupilot_methodic_configurator/backend_filesystem.py

@@ -433,11 +433,8 @@ def annotate_intermediate_comments_to_param_dict(self, param_dict: dict[str, flo
           ParDict: A ParDict of parameters with intermediate parameter file comments.
 
         """
-        ret = ParDict()
         ip_comments = self.__all_intermediate_parameter_file_comments()
-        for param, value in param_dict.items():
-            ret[param] = Par(float(value), ip_comments.get(param, ""))
-        return ret
+        return ParDict.from_float_dict(param_dict).annotate_with_comments(ip_comments)
 
     def categorize_parameters(self, param: ParDict) -> tuple[ParDict, ParDict, ParDict]:
         """
@@ -456,25 +453,7 @@ def categorize_parameters(self, param: ParDict) -> tuple[ParDict, ParDict, ParDi
                                   Each ParDict represents one of the categories mentioned above.
 
         """
-        non_default__read_only_params = ParDict()
-        non_default__writable_calibrations = ParDict()
-        non_default__writable_non_calibrations = ParDict()
-        for param_name, param_info in param.items():
-            if param_name in self.param_default_dict and is_within_tolerance(
-                param_info.value, self.param_default_dict[param_name].value
-            ):
-                continue  # parameter has a default value, ignore it
-
-            if param_name in self.doc_dict and self.doc_dict[param_name].get("ReadOnly", False):
-                non_default__read_only_params[param_name] = param_info
-                continue
-
-            if param_name in self.doc_dict and self.doc_dict[param_name].get("Calibration", False):
-                non_default__writable_calibrations[param_name] = param_info
-                continue
-            non_default__writable_non_calibrations[param_name] = param_info
-
-        return non_default__read_only_params, non_default__writable_calibrations, non_default__writable_non_calibrations
+        return param.categorize_by_documentation(self.doc_dict, self.param_default_dict, is_within_tolerance)
 
     @staticmethod
     def get_directory_name_from_full_path(full_path: str) -> str:

ardupilot_methodic_configurator/data_model_par_dict.py

@@ -15,7 +15,7 @@
 from os import popen as os_popen
 from sys import exc_info as sys_exc_info
 from types import TracebackType
-from typing import Optional, Union
+from typing import Callable, Optional, Union
 
 from ardupilot_methodic_configurator import _
 
@@ -107,14 +107,14 @@ def load_param_file_into_dict(param_file: str) -> "ParDict":
                         msg = f"Missing parameter-value separator: {line} in {param_file} line {i}"
                         raise SystemExit(msg)
                     parameter = parameter.strip()
-                    ParDict.validate_parameter(param_file, parameter_dict, i, original_line, comment, parameter, value)
+                    ParDict._validate_parameter(param_file, parameter_dict, i, original_line, comment, parameter, value)
         except UnicodeDecodeError as exp:
             msg = f"Fatal error reading {param_file}: {exp}"
             raise SystemExit(msg) from exp
         return parameter_dict
 
     @staticmethod
-    def validate_parameter(  # pylint: disable=too-many-arguments, too-many-positional-arguments
+    def _validate_parameter(  # pylint: disable=too-many-arguments, too-many-positional-arguments
         param_file: str,
         parameter_dict: "ParDict",
         i: int,
@@ -162,7 +162,7 @@ def missionplanner_sort(item: str) -> tuple[str, ...]:
         # Compare the parts separately
         return tuple(parts)
 
-    def format_params(self, file_format: str = "missionplanner") -> list[str]:  # pylint: disable=too-many-branches
+    def _format_params(self, file_format: str = "missionplanner") -> list[str]:  # pylint: disable=too-many-branches
         """
         Formats the parameters in this dictionary into a list of strings.
 
@@ -217,7 +217,7 @@ def export_to_param(self, filename_out: str, file_format: str = "missionplanner"
             file_format: File format ("missionplanner" or "mavproxy").
 
         """
-        formatted_params = self.format_params(file_format)
+        formatted_params = self._format_params(file_format)
         with open(filename_out, "w", encoding="utf-8") as output_file:
             output_file.writelines(line + "\n" for line in formatted_params)
 
@@ -278,12 +278,16 @@ def append(self, other: "ParDict") -> None:
         for param_name, param_value in other.items():
             self[param_name] = param_value
 
-    def remove_if_similar(self, other: "ParDict") -> None:
+    def remove_if_value_is_similar(self, other: "ParDict") -> None:
         """
         Remove parameters from this dictionary if their values match those in another dictionary.
 
-        This method compares parameter values and removes parameters from the current
-        dictionary if they have the same name and value as parameters in the other dictionary.
+        This method compares only parameter values and ignores comments when determining similarity.
+        Parameters from the current dictionary are removed if they have the same name and value
+        as parameters in the other dictionary, regardless of comment differences.
+
+        This is particularly useful when comparing flight controller parameters (which have no comments)
+        with file parameters (which typically have comments).
 
         Args:
             other: Another ParDict to compare against.
@@ -300,93 +304,186 @@ def remove_if_similar(self, other: "ParDict") -> None:
         keys_to_remove = []
 
         for param_name, param_value in self.items():
-            if param_name in other and param_value == other[param_name]:
+            if param_name in other and param_value.value == other[param_name].value:
                 keys_to_remove.append(param_name)
 
         # Remove the parameters that matched
         for key in keys_to_remove:
             del self[key]
 
-    def remove_if_value_is_similar(self, other: "ParDict") -> None:
+    @classmethod
+    def from_file(cls, param_file: str) -> "ParDict":
         """
-        Remove parameters from this dictionary if their values match those in another dictionary.
+        Create a ParDict by loading from a parameter file.
 
-        This method compares only parameter values and ignores comments when determining similarity.
-        Parameters from the current dictionary are removed if they have the same name and value
-        as parameters in the other dictionary, regardless of comment differences.
+        Args:
+            param_file: Path to the parameter file.
 
-        This is particularly useful when comparing flight controller parameters (which have no comments)
-        with file parameters (which typically have comments).
+        Returns:
+            A new ParDict loaded from the file.
+
+        """
+        param_dict = ParDict.load_param_file_into_dict(param_file)
+        return cls(param_dict)
+
+    @classmethod
+    def from_float_dict(cls, param_dict: dict[str, float], default_comment: str = "") -> "ParDict":
+        """
+        Create a ParDict from a dictionary of parameter names to float values.
 
         Args:
-            other: Another ParDict to compare against.
+            param_dict: Dictionary mapping parameter names to float values.
+            default_comment: Default comment to apply to all parameters.
 
-        Raises:
-            TypeError: If other is not an ParDict instance.
+        Returns:
+            A new ParDict with Par objects created from the float values.
 
         """
-        if not isinstance(other, ParDict):
-            msg = _("Can only compare with another ParDict instance")
-            raise TypeError(msg)
+        result = cls()
+        for param_name, param_value in param_dict.items():
+            result[param_name] = Par(float(param_value), default_comment)
+        return result
 
-        # Create a list of keys to remove to avoid modifying dict during iteration
-        keys_to_remove = []
+    @classmethod
+    def from_fc_parameters(cls, fc_params: dict[str, float]) -> "ParDict":
+        """
+        Create a ParDict from flight controller parameters (dict[str, float]).
 
-        for param_name, param_value in self.items():
-            if param_name in other and param_value.value == other[param_name].value:
-                keys_to_remove.append(param_name)
+        Args:
+            fc_params: Dictionary of flight controller parameters.
 
-        # Remove the parameters that matched
-        for key in keys_to_remove:
-            del self[key]
+        Returns:
+            A new ParDict with Par objects created from the flight controller parameters.
+
+        """
+        return cls.from_float_dict(fc_params)
 
-    def copy(self) -> "ParDict":
+    def _filter_by_defaults(
+        self, default_params: "ParDict", tolerance_func: Optional[Callable[[float, float], bool]] = None
+    ) -> "ParDict":
         """
-        Create a shallow copy of the ParDict.
+        Filter out parameters that have default values within tolerance.
+
+        Args:
+            default_params: ParDict containing default parameter values.
+            tolerance_func: Function to check if values are within tolerance.
+                           If None, uses exact comparison.
 
         Returns:
-            A new ParDict with the same parameters.
+            A new ParDict containing only non-default parameters.
 
         """
-        return ParDict(dict(self))
-
-    def __repr__(self) -> str:
+        result = ParDict()
+        for param_name, param_info in self.items():
+            if param_name in default_params:
+                if tolerance_func:
+                    if not tolerance_func(param_info.value, default_params[param_name].value):
+                        result[param_name] = param_info
+                elif param_info.value != default_params[param_name].value:
+                    result[param_name] = param_info
+            else:
+                result[param_name] = param_info
+        return result
+
+    def _filter_by_readonly(self, doc_dict: dict) -> "ParDict":
         """
-        Return a string representation of the ParDict.
+        Filter parameters that are marked as read-only in the documentation.
+
+        Args:
+            doc_dict: Documentation dictionary containing parameter metadata.
 
         Returns:
-            A string representation showing the class name and parameter count.
+            A new ParDict containing only read-only parameters.
 
         """
-        return f"ParDict({len(self)} parameters)"
+        result = ParDict()
+        for param_name, param_info in self.items():
+            if param_name in doc_dict and doc_dict[param_name].get("ReadOnly", False):
+                result[param_name] = param_info
+        return result
+
+    def _filter_by_calibration(self, doc_dict: dict) -> "ParDict":
+        """
+        Filter parameters that are marked as calibration parameters in the documentation.
+
+        Args:
+            doc_dict: Documentation dictionary containing parameter metadata.
+
+        Returns:
+            A new ParDict containing only calibration parameters.
 
-    def __str__(self) -> str:
         """
-        Return a human-readable string representation of the ParDict.
+        result = ParDict()
+        for param_name, param_info in self.items():
+            if param_name in doc_dict and doc_dict[param_name].get("Calibration", False):
+                result[param_name] = param_info
+        return result
+
+    def categorize_by_documentation(
+        self,
+        doc_dict: dict,
+        default_params: "ParDict",
+        tolerance_func: Optional[Callable[[float, float], bool]] = None,
+    ) -> tuple["ParDict", "ParDict", "ParDict"]:
+        """
+        Categorize parameters into read-only, calibration, and other non-default parameters.
+
+        Args:
+            doc_dict: Documentation dictionary containing parameter metadata.
+            default_params: ParDict containing default parameter values.
+            tolerance_func: Function to check if values are within tolerance.
 
         Returns:
-            A string showing the parameter count and first few parameter names.
+            A tuple of three ParDict objects:
+            - Non-default read-only parameters
+            - Non-default writable calibration parameters
+            - Non-default writable non-calibration parameters
 
         """
-        param_count = len(self)
-        if param_count == 0:
-            return "ParDict(empty)"
+        non_default_params = self._filter_by_defaults(default_params, tolerance_func)
 
-        param_names = list(self.keys()) if param_count <= 3 else [*list(self.keys())[:3], "..."]
+        # there are protected members from a locally created object, so it is OK to access them like this
+        read_only_params = non_default_params._filter_by_readonly(doc_dict)  # pylint: disable=protected-access # noqa: SLF001
+        calibration_params = non_default_params._filter_by_calibration(doc_dict)  # pylint: disable=protected-access # noqa: SLF001
 
-        return f"ParDict({param_count} parameters: {', '.join(param_names)})"
+        # Non-calibration parameters are those that are not read-only and not calibration
+        other_params = ParDict()
+        for param_name, param_info in non_default_params.items():
+            if param_name not in read_only_params and param_name not in calibration_params:
+                other_params[param_name] = param_info
 
-    @classmethod
-    def from_file(cls, param_file: str) -> "ParDict":
+        return read_only_params, calibration_params, other_params
+
+    def get_missing_or_different(self, other: "ParDict") -> "ParDict":
         """
-        Create a ParDict by loading from a parameter file.
+        Get parameters that are missing in the other ParDict or have different values.
 
         Args:
-            param_file: Path to the parameter file.
+            other: The ParDict to compare against.
 
         Returns:
-            A new ParDict loaded from the file.
+            A new ParDict containing parameters that are missing or different.
 
         """
-        param_dict = ParDict.load_param_file_into_dict(param_file)
-        return cls(param_dict)
+        result = ParDict()
+        for param_name, param in self.items():
+            if param_name not in other or other[param_name].value != param.value:
+                result[param_name] = param
+        return result
+
+    def annotate_with_comments(self, comment_lookup: dict[str, str]) -> "ParDict":
+        """
+        Create a new ParDict with comments added from a lookup table.
+
+        Args:
+            comment_lookup: Dictionary mapping parameter names to their comments.
+
+        Returns:
+            A new ParDict with updated comments.
+
+        """
+        result = ParDict()
+        for param_name, param in self.items():
+            new_comment = comment_lookup.get(param_name, param.comment or "")
+            result[param_name] = Par(param.value, new_comment)
+        return result

ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py

@@ -856,19 +856,14 @@ def _export_fc_params_missing_or_different_in_amc_files(self) -> None:
                 compound.append(ParDict(file_params))
 
         # Create FC parameters dictionary
-        fc_parameters = ParDict()
-        for param_name, param_value in self.flight_controller.fc_parameters.items():
-            fc_parameters[param_name] = Par(param_value)
+        fc_parameters = ParDict.from_fc_parameters(self.flight_controller.fc_parameters)
 
         # Remove default parameters from FC parameters if default file exists
         if "00_default.param" in self.local_filesystem.file_parameters:
             fc_parameters.remove_if_value_is_similar(ParDict(self.local_filesystem.file_parameters["00_default.param"]))
 
         # Calculate parameters that only exist in fc_parameters or have a different value from compound
-        params_missing_in_the_amc_param_files = ParDict()
-        for param_name, param in fc_parameters.items():
-            if param_name not in compound or compound[param_name].value != param.value:
-                params_missing_in_the_amc_param_files[param_name] = param
+        params_missing_in_the_amc_param_files = fc_parameters.get_missing_or_different(compound)
 
         # Export to file if there are any missing/different parameters
         if params_missing_in_the_amc_param_files:

tests/test_annotate_params.py

@@ -43,7 +43,7 @@
 )
 from ardupilot_methodic_configurator.data_model_par_dict import Par, ParDict
 
-# pylint: disable=too-many-lines
+# pylint: disable=too-many-lines, protected-access
 
 
 @pytest.fixture
@@ -717,23 +717,23 @@ def test_par_class_methods(self) -> None:
                 ParDict.load_param_file_into_dict(tf.name)
 
     def test_format_params_methods(self) -> None:
-        """Test ParDict.format_params method."""
+        """Test ParDict._format_params method."""
         param_dict = ParDict({"PARAM1": Par(100.0, "comment1"), "PARAM2": Par(200.0), "PARAM3": Par(300.0)})
 
         # Test MissionPlanner format
-        mp_format = param_dict.format_params("missionplanner")
+        mp_format = param_dict._format_params("missionplanner")
         assert any("PARAM1,100" in line for line in mp_format)
         assert any("# comment1" in line for line in mp_format)
 
         # Test MAVProxy format
-        mavproxy_format = param_dict.format_params("mavproxy")
+        mavproxy_format = param_dict._format_params("mavproxy")
         # Use correct spacing format - 16 chars for name, 8 for value
         assert any("PARAM1           100.000000" in line for line in mavproxy_format)
         assert any("# comment1" in line for line in mavproxy_format)
 
         # Test invalid format
         with pytest.raises(SystemExit):
-            param_dict.format_params("invalid_format")
+            param_dict._format_params("invalid_format")
 
 
 class AnnotateParamsTest(unittest.TestCase):