Mobt 157 nbhood refactor consolidate unit tests part1 (metoppv#1665)

* Make options for neighbourhood sum more consistent.

* Consolidate percentile from neighbourhood unit tests

* Made weighted mode only a valid option if you are using circular kernel

* Minor changes following review comments

* Remove print statement

improver/cli/nbhood.py

@@ -132,8 +132,6 @@ def process(
     from improver.utilities.pad_spatial import remove_cube_halo
     from improver.wind_calculations.wind_direction import WindDirection
 
-    sum_or_fraction = "sum" if area_sum else "fraction"
-
     if neighbourhood_output == "percentiles":
         if weighted_mode:
             raise RuntimeError(
@@ -160,7 +158,7 @@ def process(
             radius_or_radii,
             lead_times=lead_times,
             weighted_mode=weighted_mode,
-            sum_or_fraction=sum_or_fraction,
+            sum_only=area_sum,
             re_mask=remask,
         )(cube, mask_cube=mask)
     elif neighbourhood_output == "percentiles":

improver/cli/nbhood_iterate_with_mask.py

@@ -102,16 +102,14 @@ def process(
     from improver.nbhood import radius_by_lead_time
     from improver.nbhood.use_nbhood import ApplyNeighbourhoodProcessingWithAMask
 
-    sum_or_fraction = "sum" if area_sum else "fraction"
-
     radius_or_radii, lead_times = radius_by_lead_time(radii, lead_times)
 
     result = ApplyNeighbourhoodProcessingWithAMask(
         coord_for_masking,
         radius_or_radii,
         lead_times=lead_times,
         collapse_weights=weights,
-        sum_or_fraction=sum_or_fraction,
+        sum_only=area_sum,
         re_mask=remask,
     )(cube, mask)
 

improver/cli/nbhood_land_and_sea.py

@@ -106,8 +106,6 @@ def process(
     from improver.nbhood.square_kernel import NeighbourhoodProcessing
     from improver.nbhood.use_nbhood import ApplyNeighbourhoodProcessingWithAMask
 
-    sum_or_fraction = "sum" if area_sum else "fraction"
-
     masking_coordinate = None
     if any(
         "topographic_zone" in coord.name() for coord in mask.coords(dim_coords=True)
@@ -174,7 +172,7 @@ def process(
                 "square",
                 radius_or_radii,
                 lead_times=lead_times,
-                sum_or_fraction=sum_or_fraction,
+                sum_only=area_sum,
                 re_mask=True,
             )(cube, land_only)
         else:
@@ -183,7 +181,7 @@ def process(
                 radius_or_radii,
                 lead_times=lead_times,
                 collapse_weights=weights,
-                sum_or_fraction=sum_or_fraction,
+                sum_only=area_sum,
                 re_mask=False,
             )(cube, mask)
         result = result_land
@@ -194,7 +192,7 @@ def process(
             "square",
             radius_or_radii,
             lead_times=lead_times,
-            sum_or_fraction=sum_or_fraction,
+            sum_only=area_sum,
             re_mask=True,
         )(cube, sea_only)
         result = result_sea

improver/nbhood/square_kernel.py

@@ -60,8 +60,8 @@ def __init__(
         neighbourhood_method: str,
         radii: Union[float, List[float]],
         lead_times: Optional[List] = None,
-        weighted_mode: bool = True,
-        sum_or_fraction: str = "fraction",
+        weighted_mode: bool = False,
+        sum_only: bool = False,
         re_mask: bool = True,
     ) -> None:
         """
@@ -84,13 +84,8 @@ def __init__(
                 If True, use a circle for neighbourhood kernel with
                 weighting decreasing with radius.
                 If False, use a circle with constant weighting.
-            sum_or_fraction:
-                Identifier for whether sum or fraction should be returned from
-                neighbourhooding. The sum represents the sum of the
-                neighbourhood. The fraction represents the sum of the
-                neighbourhood divided by the neighbourhood area.
-                "fraction" is the default.
-                Valid options are "sum" or "fraction".
+            sum_only:
+                If true, return neighbourhood sum instead of mean.
             re_mask:
                 If re_mask is True, the original un-neighbourhood processed
                 mask is applied to mask out the neighbourhood processed cube.
@@ -105,23 +100,18 @@ def __init__(
         else:
             msg = "{} is not a valid neighbourhood_method.".format(neighbourhood_method)
             raise ValueError(msg)
-
-        self.weighted_mode = weighted_mode
-        if sum_or_fraction not in ["sum", "fraction"]:
+        if weighted_mode and neighbourhood_method != "circular":
             msg = (
-                "The neighbourhood output can either be in the form of a "
-                "sum of all the points in the neighbourhood or a fraction "
-                "of the sum of the neighbourhood divided by the "
-                "neighbourhood area. The {} option is invalid. "
-                "Valid options are 'sum' or 'fraction'."
+                "weighted_mode can only be used if neighbourhood_method is circular."
+                f" weighted_mode provided: {weighted_mode}, "
+                f"neighbourhood_method provided: {neighbourhood_method}."
             )
             raise ValueError(msg)
-        self.sum_or_fraction = sum_or_fraction
+        self.weighted_mode = weighted_mode
+        self.sum_only = sum_only
         self.re_mask = re_mask
 
-    def _calculate_neighbourhood(
-        self, data: ndarray, mask: ndarray, sum_only: bool, re_mask: bool
-    ) -> ndarray:
+    def _calculate_neighbourhood(self, data: ndarray, mask: ndarray) -> ndarray:
         """
         Apply neighbourhood processing.
 
@@ -130,17 +120,12 @@ def _calculate_neighbourhood(
                 Input data array.
             mask:
                 Mask of valid input data elements.
-            sum_only:
-                If true, return neighbourhood sum instead of mean.
-            re_mask:
-                If true, reapply the original mask and return
-                `numpy.ma.MaskedArray`.
 
         Returns:
             Array containing the smoothed field after the square
             neighbourhood method has been applied.
         """
-        if not sum_only:
+        if not self.sum_only:
             min_val = np.nanmin(data)
             max_val = np.nanmax(data)
 
@@ -176,7 +161,7 @@ def _calculate_neighbourhood(
             data = boxsum(data, self.nb_size, mode="constant")
         elif self.neighbourhood_method == "circular":
             data = correlate(data, self.kernel, mode="nearest")
-        if not sum_only:
+        if not self.sum_only:
             # Calculate neighbourhood totals for mask.
             if self.neighbourhood_method == "square":
                 area_sum = boxsum(area_mask, self.nb_size, mode="constant")
@@ -199,7 +184,7 @@ def _calculate_neighbourhood(
             data_dtype = np.float32
         data = data.astype(data_dtype)
 
-        if re_mask:
+        if self.re_mask:
             data = np.ma.masked_array(data, data_mask, copy=False)
 
         return data
@@ -250,10 +235,7 @@ def process(self, cube: Cube, mask_cube: Optional[Cube] = None) -> Cube:
         result_slices = iris.cube.CubeList()
         for cube_slice in cube.slices([cube.coord(axis="y"), cube.coord(axis="x")]):
             cube_slice.data = self._calculate_neighbourhood(
-                cube_slice.data,
-                mask_cube_data,
-                self.sum_or_fraction == "sum",
-                self.re_mask,
+                cube_slice.data, mask_cube_data
             )
             result_slices.append(cube_slice)
         neighbourhood_averaged_cube = result_slices.merge_cube()

improver/nbhood/use_nbhood.py

@@ -133,8 +133,8 @@ def __init__(
         radii: Union[float, List[float]],
         lead_times: Optional[List[float]] = None,
         collapse_weights: Optional[Cube] = None,
-        weighted_mode: bool = True,
-        sum_or_fraction: str = "fraction",
+        weighted_mode: bool = False,
+        sum_only: bool = False,
         re_mask: bool = False,
     ) -> None:
         """
@@ -170,13 +170,8 @@ def __init__(
                 If True, use a circle for neighbourhood kernel with
                 weighting decreasing with radius.
                 If False, use a circle with constant weighting.
-            sum_or_fraction:
-                Identifier for whether sum or fraction should be returned from
-                neighbourhooding. The sum represents the sum of the
-                neighbourhood.
-                The fraction represents the sum of the neighbourhood divided by
-                the neighbourhood area. "fraction" is the default.
-                Valid options are "sum" or "fraction".
+            sum_only:
+                If true, return neighbourhood sum instead of mean.
             re_mask:
                 If re_mask is True, the original un-neighbourhood processed
                 mask is applied to mask out the neighbourhood processed cube.
@@ -192,7 +187,7 @@ def __init__(
         self.lead_times = lead_times
         self.collapse_weights = collapse_weights
         self.weighted_mode = weighted_mode
-        self.sum_or_fraction = sum_or_fraction
+        self.sum_only = sum_only
         self.re_mask = re_mask
         # Check that if collapse_weights are provided then re_mask is set to False
         if self.collapse_weights is not None and re_mask is True:
@@ -262,7 +257,7 @@ def process(self, cube: Cube, mask_cube: Cube) -> Cube:
             self.radii,
             lead_times=self.lead_times,
             weighted_mode=self.weighted_mode,
-            sum_or_fraction=self.sum_or_fraction,
+            sum_only=self.sum_only,
             re_mask=self.re_mask,
         )
         yname = cube.coord(axis="y").name()

improver/precipitation_type/convection.py

@@ -66,7 +66,7 @@ def __init__(
         fuzzy_factor: Optional[float] = None,
         comparison_operator: str = ">",
         lead_times: Optional[List[float]] = None,
-        weighted_mode: bool = True,
+        weighted_mode: bool = False,
         use_adjacent_grid_square_differences: bool = True,
     ) -> None:
         """

improver/utilities/textural.py

@@ -139,7 +139,7 @@ def _calculate_ratio(self, cube: Cube, cube_name: str, radius: float) -> Cube:
         """
         # Calculate the potential transitions within neighbourhoods.
         potential_transitions = NeighbourhoodProcessing(
-            "square", radius, sum_or_fraction="sum"
+            "square", radius, sum_only=True
         ).process(cube)
         potential_transitions.data = 4 * potential_transitions.data
 
@@ -151,7 +151,7 @@ def _calculate_ratio(self, cube: Cube, cube_name: str, radius: float) -> Cube:
 
         # Sum the number of actual transitions within the neighbourhood.
         actual_transitions = NeighbourhoodProcessing(
-            "square", radius, sum_or_fraction="sum"
+            "square", radius, sum_only=True
         ).process(actual_transitions)
 
         # Calculate the ratio of actual to potential transitions in areas where the

improver_tests/acceptance/test_nbhood.py

@@ -77,7 +77,6 @@ def test_basic_square(tmp_path):
         "square",
         "--radii",
         "20000",
-        "--weighted-mode",
         "--output",
         output_path,
     ]
@@ -99,7 +98,6 @@ def test_masked_square(tmp_path):
         "square",
         "--radii",
         "20000",
-        "--weighted-mode",
         "--output",
         output_path,
     ]
@@ -214,7 +212,6 @@ def test_halo_radius(tmp_path):
         "square",
         "--radii",
         "20000",
-        "--weighted-mode",
         "--halo-radius=162000",
         "--output",
         output_path,

improver_tests/nbhood/circular_kernel/test_CircularNeighbourhood.py

@@ -47,20 +47,6 @@
 )
 
 
-class Test__init__(IrisTest):
-
-    """Test the init method."""
-
-    def test_sum_or_fraction(self):
-        """Test that a ValueError is raised if an invalid option is passed
-        in for sum_or_fraction."""
-
-        sum_or_fraction = "nonsense"
-        msg = "option is invalid"
-        with self.assertRaisesRegex(ValueError, msg):
-            NeighbourhoodProcessing("circular", 2000, sum_or_fraction=sum_or_fraction)
-
-
 class Test_apply_circular_kernel(IrisTest):
 
     """Test neighbourhood circular probabilities plugin."""
@@ -364,7 +350,9 @@ def test_basic(self):
         data[2, 2] = 0
         cube = set_up_variable_cube(data, spatial_grid="equalarea",)
 
-        result = NeighbourhoodProcessing("circular", self.RADIUS).process(cube)
+        result = NeighbourhoodProcessing(
+            "circular", self.RADIUS, weighted_mode=True
+        ).process(cube)
         self.assertIsInstance(cube, Cube)
         self.assertArrayAlmostEqual(result.data, expected_data)