From 56795a57a9571c4ee3207f96aa2221ba4f109870 Mon Sep 17 00:00:00 2001
From: Landung 'Don' Setiawan <landungs@uw.edu>
Date: Wed, 20 Sep 2023 17:20:32 -0700
Subject: [PATCH] refactor: Change 'parse_range_bin' to 'parse_x_bin'

Make bin parsing to be more general by making it
to 'parse_x_bin'.
---
 echopype/commongrid/api.py            | 62 +++++++++++++++++++++------
 echopype/tests/commongrid/test_api.py | 34 ++++++++-------
 2 files changed, 67 insertions(+), 29 deletions(-)

diff --git a/echopype/commongrid/api.py b/echopype/commongrid/api.py
index b4cddadc6..38a5f7a4b 100644
--- a/echopype/commongrid/api.py
+++ b/echopype/commongrid/api.py
@@ -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))
@@ -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
diff --git a/echopype/tests/commongrid/test_api.py b/echopype/tests/commongrid/test_api.py
index 3a75dbcd5..b0276ca72 100644
--- a/echopype/tests/commongrid/test_api.py
+++ b/echopype/tests/commongrid/test_api.py
@@ -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