Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge code improvements from Kokroo #280

Merged
merged 10 commits into from
Dec 23, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
27 changes: 10 additions & 17 deletions ncompare/sequence_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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
28 changes: 7 additions & 21 deletions ncompare/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 '.'")

Check warning on line 43 in ncompare/utils.py

View check run for this annotation

Codecov / codecov/patch

ncompare/utils.py#L43

Added line #L43 was not covered by tests
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)))
Loading