Merge pull request #293 from nsidc/issue-292

Issue-292

Author: juliacollins

CHANGELOG.md

@@ -1,3 +1,8 @@
+## UNRELEASED
+
+* Issue 292: Exclude `.spo` files from polygon-generation logic, regardless of
+  the value of the `spatial_polygon_enabled` flag in the `ini` file.
+
 ## v1.12.0 (2025-09-15)
 
 * Minor version release encompassing all features in v1.12.0rc0 - v1.12.0rc3.

src/nsidc/metgen/metgen.py

@@ -41,7 +41,6 @@
 from nsidc.metgen.collection_metadata import get_collection_metadata
 from nsidc.metgen.models import CollectionMetadata
 from nsidc.metgen.readers import registry, utilities
-from nsidc.metgen.spatial import create_flightline_polygon
 
 # -------------------------------------------------------------------
 CONSOLE_FORMAT = "%(message)s"
@@ -742,9 +741,7 @@ def create_ummg(configuration: config.Config, granule: Granule) -> Granule:
     )
 
     # Get spatial coverage from spatial file if it exists
-    spatial_content = utilities.external_spatial_values(
-        configuration.collection_geometry_override, gsr, granule
-    )
+    spatial_content = utilities.external_spatial_values(configuration, gsr, granule)
 
     # Populated summary dict looks like:
     # {
@@ -767,9 +764,7 @@ def create_ummg(configuration: config.Config, granule: Granule) -> Granule:
         gsr,
     )
 
-    summary["spatial_extent"] = populate_spatial(
-        gsr, summary["geometry"], configuration, spatial_content
-    )
+    summary["spatial_extent"] = populate_spatial(gsr, summary["geometry"])
     summary["temporal_extent"] = populate_temporal(summary["temporal"])
     summary["additional_attributes"] = populate_additional_attributes(
         premet_content, constants.UMMG_ADDITIONAL_ATTRIBUTES
@@ -983,60 +978,10 @@ def checksum(file):
 def populate_spatial(
     spatial_representation: str,
     spatial_values: list,
-    configuration: config.Config = None,
-    spatial_content: list = None,
 ) -> str:
     """
     Return a string representation of a geometry (point, bounding box, gpolygon)
-    Optionally generates optimized polygons when spatial files are present.
-    """
-    # Check if we should generate polygons for spatial file data
-    if (
-        configuration is not None
-        and configuration.spatial_polygon_enabled
-        and spatial_content is not None
-        and spatial_representation == constants.GEODETIC
-        and len(spatial_values) >= 3
-    ):
-        try:
-            # Create configured polygon generator using partial application
-            generate_polygon = partial(
-                create_flightline_polygon,
-                target_coverage=configuration.spatial_polygon_target_coverage,
-                max_vertices=configuration.spatial_polygon_max_vertices,
-                cartesian_tolerance=configuration.spatial_polygon_cartesian_tolerance,
-            )
-
-            # Extract lon/lat arrays from spatial_values
-            lons = [point["Longitude"] for point in spatial_values]
-            lats = [point["Latitude"] for point in spatial_values]
-
-            # Generate polygon using our configured spatial module
-            polygon, metadata = generate_polygon(lons, lats)
-
-            if polygon is not None:
-                # Convert shapely polygon to UMM-G format
-                coords = list(polygon.exterior.coords)
-                polygon_points = [
-                    {"Longitude": float(lon), "Latitude": float(lat)}
-                    for lon, lat in coords
-                ]
-
-                # Use the existing template system
-                return ummg_spatial_gpolygon_template().safe_substitute(
-                    {"points": json.dumps(polygon_points)}
-                )
-
-        except Exception as e:
-            # If polygon generation fails, fall back to original behavior
-            import logging
-
-            logger = logging.getLogger(constants.ROOT_LOGGER)
-            logger.warning(
-                f"Polygon generation failed, using original spatial processing: {e}"
-            )
-
-    # Original behavior for all other cases
+    """
     match spatial_representation:
         case constants.CARTESIAN:
             return populate_bounding_rectangle(spatial_values)

src/nsidc/metgen/readers/utilities.py

@@ -2,9 +2,10 @@
 from datetime import timezone
 
 from dateutil.parser import parse
-from funcy import distinct, first, last, lkeep
+from funcy import distinct, first, last, lkeep, partial
 
-from nsidc.metgen import constants
+from nsidc.metgen import config, constants
+from nsidc.metgen.spatial import create_flightline_polygon
 
 
 def temporal_from_premet(pdict: dict) -> list:
@@ -184,19 +185,24 @@ def refine_temporal(tvals: list):
     return tvals
 
 
-def external_spatial_values(collection_geometry_override, gsr, granule) -> list:
+def external_spatial_values(configuration, gsr, granule) -> list:
     """
-    Retrieve spatial information from a granule-specific spatial (or spo) file, or
+    Retrieve spatial information from a granule-specific spatial or spo file, or
     the collection metadata.
     """
-    if collection_geometry_override:
+    if configuration.collection_geometry_override:
         # Get spatial coverage from collection
         return points_from_collection(granule.collection.spatial_extent)
 
-    return points_from_spatial(granule.spatial_filename, gsr)
+    return points_from_spatial(granule.spatial_filename, gsr, configuration)
 
 
-def points_from_spatial(spatial_path: str, gsr: str) -> list:
+# TODO: Rename methods and parameters as needed to reduce overloading of the term "spatial."
+# Clarify whether we're talking about a spatial file, a spo file, or "spatial content" more
+# generically.
+def points_from_spatial(
+    spatial_path: str, gsr: str, configuration: config.Config = None
+) -> list:
     """
     Read (lon, lat) points from a .spatial or .spo file.
     """
@@ -206,30 +212,85 @@ def points_from_spatial(spatial_path: str, gsr: str) -> list:
             "spatial_dir is specified but no .spatial or .spo file exists for granule."
         )
 
+    # If spatial_path doesn't exist, then spatial information is assumed to be available
+    # in the granule data file or via collection metadata.
     if spatial_path is None:
         return None
 
     points = raw_points(spatial_path)
+    if not points:
+        raise Exception(f"No spatial values found in {spatial_path}.")
 
     # TODO: We really only need to do the "spo vs spatial" check once, since the same
     # file type will (should) be used for all granules.
     if re.search(constants.SPO_SUFFIX, spatial_path):
         return parse_spo(gsr, points)
 
+    else:
+        points = parse_spatial(points, configuration)
+
     # confirm the number of points makes sense for this granule spatial representation
     if not valid_spatial_config(gsr, len(points)):
         raise Exception(
             f"Unsupported combination of {gsr} and point count of {len(points)}."
         )
 
-    # TODO: Handle point cloud creation here if point count is greater than 1 and gsr
-    # is geodetic. Note! Flight line files can be huge!
     return points
 
 
+def parse_spatial(spatial_values: list, configuration: config.Config = None):
+    # If only a single point, or two points (assumed to identify a bounding rectangle),
+    # return spatial values without further processing
+    if len(spatial_values) <= 2:
+        return spatial_values
+
+    # Generate polygon from spatial file data
+    if configuration is not None and configuration.spatial_polygon_enabled:
+        try:
+            # Create configured polygon generator using partial application
+            generate_polygon = partial(
+                create_flightline_polygon,
+                target_coverage=configuration.spatial_polygon_target_coverage,
+                max_vertices=configuration.spatial_polygon_max_vertices,
+                cartesian_tolerance=configuration.spatial_polygon_cartesian_tolerance,
+            )
+
+            # Extract lon/lat arrays from spatial_values
+            lons = [point["Longitude"] for point in spatial_values]
+            lats = [point["Latitude"] for point in spatial_values]
+
+            # Generate polygon using our configured spatial module
+            polygon, metadata = generate_polygon(lons, lats)
+
+            if polygon is not None:
+                coords = list(polygon.exterior.coords)
+                polygon_points = [
+                    {"Longitude": float(lon), "Latitude": float(lat)}
+                    for lon, lat in coords
+                ]
+                return polygon_points
+
+        except Exception as e:
+            import logging
+
+            logger = logging.getLogger(constants.ROOT_LOGGER)
+            logger.error(f"Polygon generation failed: {e}")
+
+    # Configuration does not exist, or polygon processing is not enabled.
+    # Return values without further processing
+    return spatial_values
+
+
 def valid_spatial_config(gsr: str, point_count: int) -> str:
-    if (gsr == constants.CARTESIAN) and (point_count == 2):
-        return True
+    if not point_count:
+        return False
+
+    if point_count == 2:
+        if gsr == constants.CARTESIAN:
+            return True
+
+        else:
+            return False
 
     if gsr == constants.GEODETIC:
         return True
@@ -239,10 +300,10 @@ def valid_spatial_config(gsr: str, point_count: int) -> str:
 
 def parse_spo(gsr: str, points: list) -> list:
     """
-    Read points from a .spo file, reverse the order of the points to comply with
-    the Cumulus requirement for a clockwise order to polygon points, and ensure
-    the polygon is closed. Raise an exception if either the granule spatial representation
-    or the number of points don't support a gpolygon.
+    Reverse the order of the points to comply with the Cumulus requirement for a
+    clockwise order to polygon points, and ensure the polygon is closed. Raise an
+    exception if either the granule spatial representation or the number of
+    points don't support a gpolygon.
     """
     if gsr == constants.CARTESIAN:
         raise Exception(

tests/conftest.py

@@ -1,5 +1,7 @@
 """Shared test fixtures for the test suite."""
 
+from configparser import ConfigParser, ExtendedInterpolation
+
 import pytest
 
 from nsidc.metgen.models import CollectionMetadata
@@ -15,3 +17,23 @@ def simple_collection_metadata():
     return CollectionMetadata(
         short_name="ABCD", version="2", entry_title="Test Collection ABCD V002"
     )
+
+
+@pytest.fixture
+def cfg_parser():
+    cp = ConfigParser(interpolation=ExtendedInterpolation())
+    cp["Source"] = {"data_dir": "/data/example"}
+    cp["Collection"] = {"auth_id": "DATA-0001", "version": 42, "provider": "FOO"}
+    cp["Destination"] = {
+        "local_output_dir": "/output/here",
+        "ummg_dir": "ummg",
+        "kinesis_stream_name": "xyzzy-${environment}-stream",
+        "staging_bucket_name": "xyzzy-${environment}-bucket",
+        "write_cnm_file": False,
+    }
+    cp["Settings"] = {
+        "checksum_type": "SHA256",
+        "number": 1,
+        "log_dir": "/tmp",
+    }
+    return cp

tests/integration/configs/OLVIS1A_DUCk.ini

@@ -7,7 +7,7 @@ spatial_dir = ./data/spatial
 auth_id = OLVIS1A_DUCk
 version = 1
 provider = NSIDC_CUAT
-granule_regex = (?P<granuleid>.*_DUCK)\.JPG
+granule_regex = (?P<granuleid>.*_DUCk)\.JPG
 browse_regex = _reduced.JPG
 
 [Destination]

tests/test_config.py

@@ -1,6 +1,6 @@
 import dataclasses
 import re
-from configparser import ConfigParser, ExtendedInterpolation
+from configparser import ConfigParser
 from unittest.mock import patch
 
 import pytest
@@ -55,26 +55,6 @@ def expected_keys():
     )
 
 
-@pytest.fixture
-def cfg_parser():
-    cp = ConfigParser(interpolation=ExtendedInterpolation())
-    cp["Source"] = {"data_dir": "/data/example"}
-    cp["Collection"] = {"auth_id": "DATA-0001", "version": 42, "provider": "FOO"}
-    cp["Destination"] = {
-        "local_output_dir": "/output/here",
-        "ummg_dir": "ummg",
-        "kinesis_stream_name": "xyzzy-${environment}-stream",
-        "staging_bucket_name": "xyzzy-${environment}-bucket",
-        "write_cnm_file": False,
-    }
-    cp["Settings"] = {
-        "checksum_type": "SHA256",
-        "number": 1,
-        "log_dir": "/tmp",
-    }
-    return cp
-
-
 def test_config_parser_without_filename():
     with pytest.raises(ValueError):
         config.config_parser_factory(None)

tests/test_utilities.py

@@ -3,7 +3,7 @@
 
 import pytest
 
-from nsidc.metgen import constants, metgen
+from nsidc.metgen import config, constants, metgen
 from nsidc.metgen.readers import utilities
 
 # Unit tests for the 'utilities' module functions.
@@ -294,12 +294,14 @@ def test_spo_is_geodetic():
 
 @patch("nsidc.metgen.readers.utilities.points_from_collection")
 def test_uses_spatial_from_collection(
-    collection_handler_mock, collection_spatial, simple_collection_metadata
+    collection_handler_mock, collection_spatial, simple_collection_metadata, cfg_parser
 ):
     simple_collection_metadata.spatial_extent = collection_spatial
     fake_granule = metgen.Granule("fake_granule", collection=simple_collection_metadata)
+    cfg_parser["Source"] = {"collection_geometry_override": True}
+    cfg = config.configuration(cfg_parser, {}, constants.DEFAULT_CUMULUS_ENVIRONMENT)
 
-    utilities.external_spatial_values(True, constants.CARTESIAN, fake_granule)
+    utilities.external_spatial_values(cfg, constants.CARTESIAN, fake_granule)
     assert collection_handler_mock.called
     assert collection_handler_mock.call_args.args[0] == collection_spatial