Merge pull request #280 from nasa/feature/kokroo-code-improvements

Merge code improvements from Kokroo

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Provide numerical output where zero means no differences found ([#278](https://github.com/nasa/ncompare/pull/278)) ([**@danielfromearth**](https://github.com/danielfromearth))
 
 ### Changed
+- Code readability and CPU branching improvements [Time/Location stamp: Ursynow, Warsaw at 15:23 21/12/2024 UTC] ([#264](https://github.com/nasa/ncompare/pull/264)) ([**@kokroo**](https://github.com/kokroo))
 
 - Clean up docstrings, especially removing types that are already annotated in function signature ([#274](https://github.com/nasa/ncompare/issues/274)) ([**@danielfromearth**](https://github.com/danielfromearth))
 

ncompare/sequence_operations.py

@@ -49,24 +49,17 @@ def common_elements(
     str
         item from sequence_b, or an empty string
     """
-    a_sorted = sorted(map(coerce_to_str, sequence_a))
-    b_sorted = sorted(map(coerce_to_str, sequence_b))
-    all_items = sorted(set(a_sorted).union(set(b_sorted)))
+    # Use sets for faster membership checking
+    a_set = set(map(coerce_to_str, sequence_a))
+    b_set = set(map(coerce_to_str, sequence_b))
 
-    for i, item in enumerate(all_items):
-        item_a = item
-        item_b = item
-        if (item not in a_sorted) and (item not in b_sorted):
-            raise ValueError(
-                "Unexpected condition where an item was not found "
-                "but all items should exist in at least one list."
-            )
-
-        if item not in a_sorted:
-            item_a = ""
-        elif item not in b_sorted:
-            item_b = ""
+    # Sort the union of both sets
+    all_items = sorted(a_set | b_set)
 
+    for i, item in enumerate(all_items):
+        # Determine presence in each set
+        item_a = item if item in a_set else ""
+        item_b = item if item in b_set else ""
         yield i, item_a, item_b
 
 
@@ -96,6 +89,6 @@ def count_diffs(
     # The number of differences is computed.
     left = len(set_a - set_b)
     right = len(set_b - set_a)
-    shared = len(set_a.intersection(set_b))
+    shared = len(set_a & set_b)
 
     return left, right, shared

ncompare/utils.py

@@ -31,35 +31,21 @@
 
 def ensure_valid_path_exists(should_be_path: Union[str, Path]) -> Path:
     """Coerce input to a pathlib.Path and check that the resulting filepath exists."""
-    path_obj = _coerce_str_or_path_to_path(should_be_path)
+    path_obj = Path(should_be_path)
     if path_obj.exists():
         return path_obj
-    raise FileNotFoundError("Expected file does not exist: " + str(should_be_path))
+    raise FileNotFoundError(f"Expected file does not exist: {should_be_path}")
 
 
 def ensure_valid_path_with_suffix(should_be_path: Union[str, Path], suffix: str) -> Path:
     """Coerce input to a pathlib.Path with given suffix."""
-    path_obj = _coerce_str_or_path_to_path(should_be_path)
-    return path_obj.with_suffix(suffix)
+    if not suffix.startswith("."):
+        raise ValueError(f"Invalid suffix: {suffix}. It must start with '.'")
+    return Path(should_be_path).with_suffix(suffix)
 
 
-def coerce_to_str(some_object: Union[str, int, tuple]):
+def coerce_to_str(some_object: Union[str, int, tuple]) -> str:
     """Ensure the type is a string."""
-    if isinstance(some_object, str):
-        return some_object
-    if isinstance(some_object, int):
+    if isinstance(some_object, (str, int, tuple)):
         return str(some_object)
-    if isinstance(some_object, tuple):
-        return str(some_object)
-
     raise TypeError(f"Unable to coerce value to str. Unexpected type <{type(some_object)}>.")
-
-
-def _coerce_str_or_path_to_path(should_be_path: Union[Path, str]) -> Path:
-    wrong_type_msg = "Unexpected type for something that should be convertable to a Path: "
-    if isinstance(should_be_path, str):
-        return Path(should_be_path)
-    elif isinstance(should_be_path, Path):
-        return should_be_path
-    else:
-        raise TypeError(wrong_type_msg + str(type(should_be_path)))