Various fixes stemming from self review of the PR.

.yamllint.yml

@@ -1,5 +1,4 @@
 extends: default
-# ignore: []
 
 rules:
   indentation:

hier_config/__init__.py

@@ -6,12 +6,14 @@
     get_hconfig_from_dump,
     get_hconfig_view,
 )
+from .model import Platform
 from .root import HConfig
 from .workflow import WorkflowRemediation
 
 __all__ = (
     "HConfig",
     "HConfigChild",
+    "Platform",
     "WorkflowRemediation",
     "get_hconfig",
     "get_hconfig_driver",

hier_config/base.py

@@ -41,13 +41,14 @@ def __hash__(self) -> int:
     def __iter__(self) -> Iterator[HConfigChild]:
         return iter(self.children)
 
+    @property
     @abstractmethod
-    def _duplicate_child_allowed_check(self) -> bool:
+    def root(self) -> HConfig:
         pass
 
     @property
     @abstractmethod
-    def root(self) -> HConfig:
+    def driver(self) -> HConfigDriverBase:
         pass
 
     @abstractmethod
@@ -58,11 +59,6 @@ def lineage(self) -> Iterator[HConfigChild]:
     def depth(self) -> int:
         pass
 
-    @property
-    @abstractmethod
-    def _child_class(self) -> type[HConfigChild]:
-        pass
-
     def add_children(self, lines: Iterable[str]) -> None:
         """Add child instances of HConfigChild."""
         for line in lines:
@@ -291,8 +287,8 @@ def unified_diff(self, target: HConfig | HConfigChild) -> Iterator[str]:
                 )
 
     def _future_pre(self, config: HConfig | HConfigChild) -> tuple[set[str], set[str]]:
-        negated_or_recursed = set()
-        config_children_ignore = set()
+        negated_or_recursed: set[str] = set()
+        config_children_ignore: set[str] = set()
         for self_child in self.children:
             # Is the command effectively negating a command in self.children?
             if (
@@ -310,7 +306,6 @@ def _future(  # noqa: C901
         """The below cases still need to be accounted for:
         - negate a numbered ACL when removing an item
         - idempotent command avoid list
-        - idempotent_acl_check
         - and likely others.
         """
         negated_or_recursed, config_children_ignore = self._future_pre(config)
@@ -363,7 +358,11 @@ def _future(  # noqa: C901
 
     @property
     @abstractmethod
-    def driver(self) -> HConfigDriverBase:
+    def _child_class(self) -> type[HConfigChild]:
+        pass
+
+    @abstractmethod
+    def _duplicate_child_allowed_check(self) -> bool:
         pass
 
     def _with_tags(

hier_config/child.py

@@ -189,7 +189,7 @@ def cisco_style_text(
         tag: str | None = None,
     ) -> str:
         """Return a Cisco style formated line i.e. indentation_level + text ! comments."""
-        comments = []
+        comments: list[str] = []
         if style == "without_comments":
             pass
         elif style == "merged":
@@ -294,10 +294,6 @@ def is_idempotent_command(self, other_children: Iterable[HConfigChild]) -> bool:
             if self.lineage_test(rule.lineage):
                 return False
 
-        # Handles idempotent acl entry identification
-        if self.driver.idempotent_acl_check(self, other_children):
-            return True
-
         # Idempotent command identification
         return bool(self.driver.idempotent_for(self, other_children))
 
@@ -355,7 +351,7 @@ def line_inclusion_test(
     @property
     def instance(self) -> Instance:
         return Instance(
-            id=id(self),
+            id=id(self.root),
             comments=frozenset(self.comments),
             tags=frozenset(self.tags),
         )

hier_config/constructors.py

@@ -116,6 +116,10 @@ def get_hconfig_from_dump(dump: Dump) -> HConfig:
     return config
 
 
+def get_hconfig_fast_generic_load(lines: list[str] | tuple[str, ...] | str) -> HConfig:
+    return get_hconfig_fast_load(Platform.GENERIC, lines)
+
+
 def get_hconfig_fast_load(
     platform_or_driver: Platform | HConfigDriverBase,
     lines: list[str] | tuple[str, ...] | str,
@@ -207,7 +211,7 @@ def _load_from_string_lines(config: HConfig, config_text: str) -> None:  # noqa:
     most_recent_item: HConfig | HConfigChild = current_section
     indent_adjust = 0
     end_indent_adjust: list[str] = []
-    temp_banner = []
+    temp_banner: list[str] = []
     banner_end_lines = {"EOF", "%", "!"}
     banner_end_contains: list[str] = []
     in_banner = False

hier_config/model.py

@@ -1,7 +1,7 @@
 from enum import Enum, auto
 
 from pydantic import BaseModel as PydanticBaseModel
-from pydantic import ConfigDict, PositiveInt
+from pydantic import ConfigDict, NonNegativeInt, PositiveInt
 
 
 class BaseModel(PydanticBaseModel):
@@ -11,7 +11,7 @@ class BaseModel(PydanticBaseModel):
 
 
 class DumpLine(BaseModel):
-    depth: PositiveInt
+    depth: NonNegativeInt
     text: str
     tags: frozenset[str]
     comments: frozenset[str]

hier_config/platforms/cisco_xr/driver.py

@@ -268,20 +268,21 @@ class HConfigDriverCiscoIOSXR(HConfigDriverBase):  # pylint: disable=too-many-in
         ),
     )
 
-    @staticmethod
-    def idempotent_acl_check(
+    def idempotent_for(
+        self,
         config: HConfigChild,
         other_children: Iterable[HConfigChild],
-    ) -> bool:
+    ) -> HConfigChild | None:
         if isinstance(config.parent, HConfigChild):
             acl = ("ipv4 access-list ", "ipv6 access-list ")
             if config.parent.text.startswith(acl):
                 self_sn = config.text.split(" ", 1)[0]
                 for other_child in other_children:
                     other_sn = other_child.text.split(" ", 1)[0]
                     if self_sn == other_sn:
-                        return True
-        return False
+                        return other_child
+
+        return super().idempotent_for(config, other_children)
 
     @property
     def platform(self) -> Platform:

hier_config/platforms/driver_base.py

@@ -43,10 +43,6 @@ class HConfigDriverBase(ABC, BaseModel):  # pylint: disable=too-many-instance-at
     negation_negate_with_rules: tuple[NegationDefaultWithRule, ...] = ()
     post_load_callbacks: tuple[Callable[[HConfig], None], ...] = ()
 
-    @staticmethod
-    def idempotent_acl_check(_: HConfigChild, __: Iterable[HConfigChild]) -> bool:
-        return False
-
     def idempotent_for(
         self,
         config: HConfigChild,

hier_config/platforms/model.py

@@ -1,6 +1,6 @@
 from enum import Enum, auto
 
-from pydantic import PositiveInt
+from pydantic import NonNegativeInt, PositiveInt
 
 from hier_config.model import BaseModel
 
@@ -10,7 +10,6 @@ class NACHostMode(str, Enum):
     SINGLE_HOST = "single-host"
     MULTI_DOMAIN = "multi-domain"
     MULTI_AUTH = "multi-auth"
-    # We don't use multi-host
     MULTI_HOST = "multi-host"
 
 
@@ -21,8 +20,8 @@ class InterfaceDot1qMode(str, Enum):
 
 
 class StackMember(BaseModel):
-    id: PositiveInt
-    priority: PositiveInt
+    id: NonNegativeInt
+    priority: NonNegativeInt
     mac_address: str | None  # not defined in cisco_ios stacks
     model: str
 

hier_config/platforms/view_base.py

@@ -146,7 +146,7 @@ def parent_name(self) -> str | None:
     @property
     @abstractmethod
     def poe(self) -> bool:
-        """Determine the PoE is enabled."""
+        """Determine if PoE is enabled."""
 
     @property
     @abstractmethod
@@ -247,7 +247,7 @@ def location(self) -> str:
 
     @property
     def module_numbers(self) -> Iterable[int]:
-        seen = set()
+        seen: set[int] = set()
         for interface_view in self.interface_views:
             if module_number := interface_view.module_number:
                 if module_number in seen:

hier_config/root.py

@@ -30,10 +30,6 @@ def __init__(self, driver: HConfigDriverBase) -> None:
         super().__init__()
         self._driver = driver
 
-    @property
-    def driver(self) -> HConfigDriverBase:
-        return self._driver
-
     def __str__(self) -> str:
         return "\n".join(str(c) for c in sorted(self.children))
 
@@ -59,6 +55,10 @@ def __eq__(self, other: object) -> bool:
             )
         )
 
+    @property
+    def driver(self) -> HConfigDriverBase:
+        return self._driver
+
     @property
     def real_indent_level(self) -> int:
         return -1
@@ -174,6 +174,7 @@ def config_to_get_to(
             delta = HConfig(self.driver)
 
         root_config = self._config_to_get_to(target, delta)
+        # Makes mypy happy
         if not isinstance(root_config, HConfig):
             raise TypeError
 

pyproject.toml

@@ -58,16 +58,6 @@ build-backend = "poetry.core.masonry.api"
 [tool.pyright]
 typeCheckingMode = "strict"
 
-reportUnusedFunction = "none"
-reportPrivateUsage = "none"
-reportUnknownParameterType = "none"
-reportUnknownLambdaType = "none"
-reportIncompatibleMethodOverride = "none"
-reportUnknownArgumentType = "none"
-reportUnknownMemberType = "none"
-reportUnknownVariableType = "none"
-reportMissingTypeStubs = "none"
-
 
 [tool.pylint.message_control]
 load-plugins = [
@@ -89,14 +79,13 @@ load-plugins = [
     "pylint_pydantic",
 ]
 disable = [
-    "bad-dunder-name",  # produces several false positives
     "broad-exception-caught",  # Covered by ruff BLE001
+    "bad-dunder-name",  # Covered by ruff PLW3201
     "consider-alternative-union-syntax",  # covered by Ruff's UP007
-    "duplicate-code",  # There are too many cases where this check hits that are difficult to address
+    "duplicate-code",  # Enable this at some point in the future
     "eq-without-hash", # Covered by ruff PLW1641
     "fixme",  # Covered by ruff FIX002
     "import-outside-toplevel", # Covered by ruff PLC0415
-    "invalid-name",
     "line-too-long",  # Whatever ruff format determines the line should look like is fine
     "missing-class-docstring",
     "missing-function-docstring",
@@ -124,12 +113,12 @@ disable = [
 #]
 
 [tool.mypy]
-ignore_missing_imports = true
+strict = true
+#ignore_missing_imports = false
 pretty = true
 show_column_numbers = true
 show_error_codes = true
 show_error_context = true
-strict = true
 warn_unreachable = true
 plugins = [
     "pydantic.mypy"
@@ -140,9 +129,12 @@ plugins = [
 line-length = 88
 target-version = "py310"
 
+[tool.ruff.format]
+docstring-code-format = true
+
 [tool.ruff.lint]
-preview = true
 select = ["ALL"]
+preview = true
 ignore = [
     # The below checks we might never enable
     "B019",  # using lru_cache can cause memory leaks

scripts/build.py

@@ -84,16 +84,6 @@ def _ruff_check_command(*, fix: bool, unsafe_fixes: bool, statistics: bool) -> s
     return f"ruff check --output-format=concise {'--fix ' if fix else ''}{'--unsafe-fixes ' if unsafe_fixes else ''}{'--statistics ' if statistics else ''}{_python_base_paths_str()}"
 
 
-@app.command()
-def black(*, fix: bool = False) -> None:
-    """Run black code formatter."""
-    _run(_black_command(fix=fix))
-
-
-def _black_command(*, fix: bool) -> str:
-    return f"black {'' if fix else '--check '}{_repo_path().relative_to(Path.cwd())}"
-
-
 @app.command()
 def pytest(
     *,
@@ -220,7 +210,7 @@ def _project_paths(glob: str) -> Iterable[Path]:
 
 
 def _run_commands_threaded(commands: tuple[str, ...]) -> NoReturn:
-    return_codes = {}
+    return_codes: dict[str, int] = {}
     with ThreadPoolExecutor(max_workers=len(commands)) as executor:
         for future in as_completed(
             executor.submit(

tests/test_driver_cisco_ios.py

@@ -15,7 +15,7 @@ def test_logging_console_emergencies_scenario_1() -> None:
     assert rollback.dump_simple() == ("no logging console",)
     running_after_rollback = future_config.future(rollback)
 
-    assert not list(running_config.unified_diff(running_after_rollback))
+    assert not tuple(running_config.unified_diff(running_after_rollback))
 
 
 def test_logging_console_emergencies_scenario_2() -> None:
@@ -30,7 +30,7 @@ def test_logging_console_emergencies_scenario_2() -> None:
     assert rollback.dump_simple() == ("logging console",)
     running_after_rollback = future_config.future(rollback)
 
-    assert not list(running_config.unified_diff(running_after_rollback))
+    assert not tuple(running_config.unified_diff(running_after_rollback))
 
 
 def test_logging_console_emergencies_scenario_3() -> None:
@@ -45,4 +45,4 @@ def test_logging_console_emergencies_scenario_3() -> None:
     assert rollback.dump_simple() == ("logging console debugging",)
     running_after_rollback = future_config.future(rollback)
 
-    assert not list(running_config.unified_diff(running_after_rollback))
+    assert not tuple(running_config.unified_diff(running_after_rollback))

tests/test_driver_hp_procurve.py

@@ -79,4 +79,4 @@ def test_future() -> None:
         ),
     )
     future_config = running_config.future(remediation_config)
-    assert not list(remediation_config.unified_diff(future_config))
+    assert not tuple(remediation_config.unified_diff(future_config))