Skip to content

Commit

Permalink
refactor: Change 'parse_range_bin' to 'parse_x_bin'
Browse files Browse the repository at this point in the history
Make bin parsing to be more general by making it
to 'parse_x_bin'.
  • Loading branch information
lsetiawan committed Sep 21, 2023
1 parent 3492b36 commit 56795a5
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 29 deletions.
62 changes: 48 additions & 14 deletions echopype/commongrid/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,41 +88,75 @@ def _convert_bins_to_interval_index(
return pd.IntervalIndex.from_breaks(bins, closed=closed).sort_values()


def _parse_range_bin(range_bin: str) -> float:
def _parse_x_bin(x_bin: str, x_label="range_bin") -> float:
"""
Parses range bin string, check unit,
and returns range bin in meters.
Parses x bin string, check unit,
and returns x bin in x unit.
Currently only available for:
range_bin: meters (m)
dist_bin: nautical miles (nmi)
Parameters
----------
range_bin : str
Range bin string, e.g., "10m"
x_bin : str
X bin string, e.g., "0.5nmi" or "10m"
x_label : {"range_bin", "dist_bin"}, default "range_bin"
The label of the x bin.
Returns
-------
float
The resulting range bin value in meters.
The resulting x bin value in x unit,
based on label.
Raises
------
ValueError
If the range bin string doesn't include 'm' for meters.
If the x bin string doesn't include unit value.
TypeError
If the range bin is not a type string.
If the x bin is not a type string.
KeyError
If the x label is not one of the available labels.
"""
x_bin_map = {
"range_bin": {
"name": "Range bin",
"unit": "m",
"ex": "10m",
"unit_label": "meters",
"pattern": r"([\d+]*[.,]{0,1}[\d+]*)(\s+)?(m)",
},
"dist_bin": {
"name": "Distance bin",
"unit": "nmi",
"ex": "0.5nmi",
"unit_label": "nautical miles",
"pattern": r"([\d+]*[.,]{0,1}[\d+]*)(\s+)?(nmi)",
},
}
x_bin_info = x_bin_map.get(x_label, None)

if x_bin_info is None:
raise KeyError(f"x_label must be one of {list(x_bin_map.keys())}")

# First check for bin types
if not isinstance(range_bin, str):
raise TypeError("range_bin must be a string")
if not isinstance(x_bin, str):
raise TypeError("'x_bin' must be a string")
# normalize to lower case
# for range_bin
range_bin = range_bin.strip().lower()
x_bin = x_bin.strip().lower()
# Only matches meters
match_obj = re.match(r"([\d+]*[.,]{0,1}[\d+]*)(\s+)?(m)", range_bin)
match_obj = re.match(x_bin_info["pattern"], x_bin)

# Do some checks on range meter inputs
if match_obj is None:
# This shouldn't be other units
raise ValueError("range_bin must be in meters (e.g., '10m').")
raise ValueError(
f"{x_bin_info['name']} must be in "
f"{x_bin_info['unit_label']} "
f"(e.g., '{x_bin_info['ex']}')."
)

# Convert back to float
range_bin = float(match_obj.group(1))
Expand Down Expand Up @@ -179,7 +213,7 @@ def compute_MVBS(
if not isinstance(ping_time_bin, str):
raise TypeError("ping_time_bin must be a string")

range_bin = _parse_range_bin(range_bin)
range_bin = _parse_x_bin(range_bin, "range_bin")

# Clean up filenames dimension if it exists
# not needed here
Expand Down
34 changes: 19 additions & 15 deletions echopype/tests/commongrid/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,32 @@

# Utilities Tests
@pytest.mark.parametrize(
["range_bin", "expected_result"],
["x_bin", "x_label", "expected_result"],
[
(5, TypeError),
("0.2m", 0.2),
("10m", 10.0),
("10km", ValueError),
("10", ValueError)
# Success
("10m", "range_bin", 10.0),
("0.2m", "range_bin", 0.2),
("0.5nmi", "dist_bin", 0.5),
# Errored
(10, "range_bin", TypeError),
("10km", "range_bin", ValueError),
("10", "range_bin", ValueError),
("10m", "invalid_label", KeyError),
],
)
def test__parse_range_bin(range_bin, expected_result):
expected_error_msg = r"range_bin must be in meters"
if isinstance(range_bin, int):
expected_error_msg = r"range_bin must be a string"
elif range_bin in ["10km", "10"]:
expected_error_msg = r"range_bin must be in meters"
def test__parse_x_bin(x_bin, x_label, expected_result):
if x_label == "invalid_label":
expected_error_msg = r"x_label must be one of"
elif isinstance(x_bin, int):
expected_error_msg = r"must be a string"
elif x_bin in ["10km", "10"]:
expected_error_msg = r"must be in"

if not isinstance(expected_result, float):
with pytest.raises(expected_result, match=expected_error_msg):
ep.commongrid.api._parse_range_bin(range_bin)
ep.commongrid.api._parse_x_bin(x_bin, x_label)
else:
assert ep.commongrid.api._parse_range_bin(range_bin) == expected_result

assert ep.commongrid.api._parse_x_bin(x_bin, x_label) == expected_result

# NASC Tests
@pytest.mark.integration
Expand Down

0 comments on commit 56795a5

Please sign in to comment.