refactor reduce duplicated code

neilSchroeder · neilSchroeder · commit 93102a9e9031 · 2025-11-07T14:03:48.000-07:00
diff --git a/climakitae/new_core/param_validation/clip_param_validator.py b/climakitae/new_core/param_validation/clip_param_validator.py
@@ -17,7 +17,10 @@
 from climakitae.new_core.param_validation.param_validation_tools import (
     _get_closest_options,
 )
-from climakitae.new_core.processors.processor_utils import is_station_identifier
+from climakitae.new_core.processors.processor_utils import (
+    find_station_match,
+    is_station_identifier,
+)
 
 
 @register_processor_validator("clip")
@@ -450,23 +453,8 @@ def _validate_station_identifier(value: str) -> bool:
             )
             return False
 
-        # Normalize the input
-        station_id_upper = value.upper().strip()
-
-        # Try exact match on ID column
-        match = stations_df[stations_df["ID"].str.upper() == station_id_upper]
-
-        if len(match) == 0:
-            # Try matching on station name (full station column)
-            match = stations_df[stations_df["station"].str.upper() == station_id_upper]
-
-        if len(match) == 0:
-            # Try partial match on station name
-            match = stations_df[
-                stations_df["station"]
-                .str.upper()
-                .str.contains(station_id_upper, na=False)
-            ]
+        # Use the generalized matching logic
+        match = find_station_match(value, stations_df)
 
         if len(match) == 0:
             # Station not found - provide suggestions from both stations and boundaries
@@ -648,7 +636,7 @@ def _warn_about_case_sensitivity(value: str) -> bool:
             hint = f"\nNote: '{value}' is all lowercase. If this is a state abbreviation, it should probably be uppercase (e.g., '{value.upper()}')."
         elif "county" in value.lower() and not value.endswith("County"):
             hint = "\nNote: County names typically end with 'County' (e.g., 'Los Angeles County')."
-        
+
         warnings.warn(
             f"\n\nBoundary key '{value}' may have case sensitivity issues. "
             f"\nDid you mean any of the following options: "
diff --git a/climakitae/new_core/processors/clip.py b/climakitae/new_core/processors/clip.py
@@ -127,7 +127,10 @@
     DataProcessor,
     register_processor,
 )
-from climakitae.new_core.processors.processor_utils import is_station_identifier
+from climakitae.new_core.processors.processor_utils import (
+    find_station_match,
+    is_station_identifier,
+)
 from climakitae.util.utils import get_closest_gridcell, get_closest_gridcells
 
 
@@ -443,23 +446,8 @@ def _get_station_coordinates(
         if stations_df is None or len(stations_df) == 0:
             raise RuntimeError("Station data is not available in the catalog.")
 
-        # Normalize the input
-        station_id_upper = station_identifier.upper().strip()
-
-        # Try exact match on ID column
-        match = stations_df[stations_df["ID"].str.upper() == station_id_upper]
-
-        if len(match) == 0:
-            # Try matching on station name (full station column)
-            match = stations_df[stations_df["station"].str.upper() == station_id_upper]
-
-        if len(match) == 0:
-            # Try partial match on station name
-            match = stations_df[
-                stations_df["station"]
-                .str.upper()
-                .str.contains(station_id_upper, na=False)
-            ]
+        # Use the generalized matching logic
+        match = find_station_match(station_identifier, stations_df)
 
         if len(match) == 0:
             # Station not found - provide suggestions
@@ -474,7 +462,7 @@ def _get_station_coordinates(
 
             error_msg = f"Station '{station_identifier}' not found in station database."
             if suggestions:
-                error_msg += f"\n\nDid you mean one of these?\n  - " + "\n  - ".join(
+                error_msg += "\n\nDid you mean one of these?\n  - " + "\n  - ".join(
                     suggestions[:5]
                 )
             error_msg += (
diff --git a/climakitae/new_core/processors/processor_utils.py b/climakitae/new_core/processors/processor_utils.py
@@ -86,6 +86,86 @@ def is_station_identifier(value: str) -> bool:
     return False
 
 
+def find_station_match(station_identifier: str, stations_df):
+    """
+    Find matching station(s) in the stations DataFrame.
+
+    This function centralizes the station matching logic used by both the Clip
+    processor and the clip parameter validator. It tries multiple matching strategies
+    in order of specificity:
+    1. Exact match on station ID column
+    2. Exact match on station name column
+    3. Partial match on station name column
+
+    Parameters
+    ----------
+    station_identifier : str
+        Station identifier to search for (e.g., "KSAC", "Sacramento (KSAC)", "Sacramento")
+    stations_df : pd.DataFrame
+        DataFrame containing station data with columns: ID, station, city, state, LAT_Y, LON_X
+
+    Returns
+    -------
+    pd.DataFrame
+        DataFrame containing matching station(s). May have 0, 1, or multiple rows:
+        - Empty (len=0): No matches found
+        - Single row (len=1): Exact match found
+        - Multiple rows (len>1): Multiple stations match the identifier
+
+    Notes
+    -----
+    The caller is responsible for:
+    - Checking if stations_df is None or empty before calling
+    - Handling the different match scenarios (no match, single match, multiple matches)
+    - Providing appropriate error messages or warnings based on context
+
+    Examples
+    --------
+    >>> # For validation (clip_param_validator.py)
+    >>> match = find_station_match("KSAC", stations_df)
+    >>> if len(match) == 0:
+    ...     # Handle no match - provide suggestions
+    >>> elif len(match) > 1:
+    ...     # Handle multiple matches - ask user to be more specific
+    >>> else:
+    ...     # Valid single match
+    ...     return True
+
+    >>> # For coordinate extraction (clip.py)
+    >>> match = find_station_match("KSAC", stations_df)
+    >>> if len(match) == 0:
+    ...     # Raise ValueError with suggestions
+    >>> elif len(match) > 1:
+    ...     # Raise ValueError asking for more specific identifier
+    >>> else:
+    ...     # Extract coordinates and metadata
+    ...     lat = float(match.iloc[0]["LAT_Y"])
+    ...     lon = float(match.iloc[0]["LON_X"])
+    """
+    # Normalize the input
+    station_id_upper = station_identifier.upper().strip()
+
+    # Handle empty string after normalization
+    if not station_id_upper:
+        # Return empty DataFrame with same structure
+        return stations_df.iloc[0:0]
+
+    # Try exact match on ID column
+    match = stations_df[stations_df["ID"].str.upper() == station_id_upper]
+
+    if len(match) == 0:
+        # Try matching on station name (full station column)
+        match = stations_df[stations_df["station"].str.upper() == station_id_upper]
+
+    if len(match) == 0:
+        # Try partial match on station name
+        match = stations_df[
+            stations_df["station"].str.upper().str.contains(station_id_upper, na=False)
+        ]
+
+    return match
+
+
 def _get_block_maxima_optimized(
     da_series: xr.DataArray,
     extremes_type: str = "max",
diff --git a/tests/new_core/param_validation/test_clip_param_validation.py b/tests/new_core/param_validation/test_clip_param_validation.py
@@ -344,7 +344,9 @@ def test_validate_list_all_items_invalid_returns_false(self, mock_validate):
         # Mock returns None for invalid items (the code checks `if validated_item is not None`)
         mock_validate.return_value = None
 
-        with pytest.warns(UserWarning, match="Found \\d+ invalid items in clip parameter list"):
+        with pytest.warns(
+            UserWarning, match="Found \\d+ invalid items in clip parameter list"
+        ):
             result = _validate_list_param(["INVALID1", "INVALID2"])
             assert result is False
 
@@ -651,9 +653,7 @@ def test_warn_no_match_warns_and_returns_false(self, mock_data_catalog_class):
             assert result is False
 
     @patch("climakitae.new_core.param_validation.clip_param_validator.DataCatalog")
-    def test_warn_lowercase_state_abbreviation_adds_hint(
-        self, mock_data_catalog_class
-    ):
+    def test_warn_lowercase_state_abbreviation_adds_hint(self, mock_data_catalog_class):
         """Test _warn_about_case_sensitivity with lowercase state abbreviation adds hint to suggestion warning."""
         mock_catalog = MagicMock()
         mock_data_catalog_class.return_value = mock_catalog
diff --git a/tests/new_core/processors/test_processor_utils.py b/tests/new_core/processors/test_processor_utils.py
@@ -31,6 +31,7 @@
     _optimize_chunking_for_block_maxima,
     _set_block_maxima_attributes,
     extend_time_domain,
+    find_station_match,
 )
 
 
@@ -1455,3 +1456,103 @@ def test_mixed_data_types(self):
         assert isinstance(result, xr.DataArray)
         # Should preserve data type characteristics
         assert result.dtype.kind in ["i", "f"]  # integer or float
+
+
+class TestFindStationMatch:
+    """Test class for find_station_match utility function."""
+
+    def setup_method(self):
+        """Set up test fixtures."""
+        # Create a mock stations DataFrame matching the structure of real station data
+        self.stations_df = pd.DataFrame(
+            {
+                "ID": ["KSAC", "KBFL", "KSFO", "KSAN", "KOAK"],
+                "station": [
+                    "Sacramento (KSAC)",
+                    "Bakersfield (KBFL)",
+                    "San Francisco International (KSFO)",
+                    "San Diego International (KSAN)",
+                    "Oakland International (KOAK)",
+                ],
+                "city": [
+                    "Sacramento",
+                    "Bakersfield",
+                    "San Francisco",
+                    "San Diego",
+                    "Oakland",
+                ],
+                "state": ["CA", "CA", "CA", "CA", "CA"],
+                "LAT_Y": [38.5, 35.4, 37.6, 32.7, 37.7],
+                "LON_X": [-121.5, -119.1, -122.4, -117.2, -122.2],
+            }
+        )
+
+    def test_exact_match_on_id(self):
+        """Test exact match on station ID."""
+        match = find_station_match("KSAC", self.stations_df)
+
+        assert len(match) == 1
+        assert match.iloc[0]["ID"] == "KSAC"
+        assert match.iloc[0]["city"] == "Sacramento"
+
+    def test_exact_match_case_insensitive(self):
+        """Test exact match is case-insensitive."""
+        match = find_station_match("ksac", self.stations_df)
+
+        assert len(match) == 1
+        assert match.iloc[0]["ID"] == "KSAC"
+
+    def test_exact_match_with_whitespace(self):
+        """Test exact match handles leading/trailing whitespace."""
+        match = find_station_match("  KSAC  ", self.stations_df)
+
+        assert len(match) == 1
+        assert match.iloc[0]["ID"] == "KSAC"
+
+    def test_exact_match_on_station_name(self):
+        """Test exact match on station name column."""
+        match = find_station_match("Sacramento (KSAC)", self.stations_df)
+
+        assert len(match) == 1
+        assert match.iloc[0]["ID"] == "KSAC"
+
+    def test_partial_match_on_station_name(self):
+        """Test partial match on station name."""
+        match = find_station_match("Sacramento", self.stations_df)
+
+        assert len(match) == 1
+        assert match.iloc[0]["ID"] == "KSAC"
+
+    def test_partial_match_multiple_results(self):
+        """Test partial match returning multiple results."""
+        # Add another station with "International" in the name
+        match = find_station_match("International", self.stations_df)
+
+        # Should match San Francisco International, San Diego International, Oakland International
+        assert len(match) == 3
+        assert all("International" in name for name in match["station"].values)
+
+    def test_no_match(self):
+        """Test when no match is found."""
+        match = find_station_match("KZZZ", self.stations_df)
+
+        assert len(match) == 0
+
+    def test_no_match_invalid_name(self):
+        """Test when station name doesn't match anything."""
+        match = find_station_match("Nonexistent City", self.stations_df)
+
+        assert len(match) == 0
+
+    def test_empty_identifier(self):
+        """Test with empty string identifier."""
+        # After strip(), empty string won't match anything
+        match = find_station_match("", self.stations_df)
+
+        assert len(match) == 0
+
+    def test_whitespace_only_identifier(self):
+        """Test with whitespace-only identifier."""
+        match = find_station_match("   ", self.stations_df)
+
+        assert len(match) == 0
