Fix/v0.2.0 bugs testing and error path detection (#10)

* fix: normalize vendor-specific JSON keys to canonical schema names

Adds tests on the resolution provided to #5 which was causing report generation to fail.

* test: scaffold pytest harness with synthetic pattern fixtures

* fix(parser): guard calculate_top_3db_point against missing -3dB crossing

* fix(parser): reject unknown source coordinate systems instead of silent no-op

* fix(parser): use equality not substring check for internal coord system

* fix(report): use relative imports to break circular dependency

* fix(parser): guard beam efficiency against zero overall power

* fix(parser): validate Data_Set is non-empty on init

* fix: replace deprecated importlib.resources APIs (removed in 3.14)

* fix(report): use module logger instead of root logger

* fix(parser): reject out-of-range theta after coordinate transform

* test(parser): pin phi-boundary consistency across coord transforms (bug 42 non-repro)

* fix(parser): use theta grid step instead of hardcoded 1deg in 3dB search

* fix(parser): reject out-of-range phi after coordinate transform

* fix(parser): raise when validation requested but schema unavailable

* fix(report): guard empty subband list before pd.concat

* fix(report): surface skipped files and fail clearly when all skipped

* fix(parser): show zero values in __str__ using is-not-None check

* refactor(parser): collapse coordinate-transform dispatch into lookup table

* style: keep string literals on single lines for greppability

* test(parser): add _normalize_json vendor-key tests (issue #5)

Author: tiamilani

pyproject.toml

@@ -59,6 +59,7 @@ nbformat = "^5.10.4"
 ruff = "^0.11.12"
 mypy = "^1.16.0"
 pre-commit = "^4.2.0"
+pytest = "^8.0"
 
 [tool.poetry.group.typing.dependencies]
 pandas-stubs = "2.2.2.240909"
@@ -154,6 +155,14 @@ indent-style = "space"
 docstring-code-format = true
 
 
+[tool.pytest.ini_options]
+minversion = "8.0"
+testpaths = ["tests"]
+addopts = "-q"
+filterwarnings = [
+    "ignore::DeprecationWarning",
+]
+
 [tool.mypy]
 python_version = "3.13"
 warn_return_any = true

src/eas_3d_pattern/parser.py

@@ -41,6 +41,18 @@
     "Theta_Tilt": "Theta_Electrical_Tilt",
 }
 
+# Coordinate transforms into the internal SPCS_Ericsson frame.
+# Each entry maps (theta, phi) arrays -> (theta, phi) arrays. The phi operator
+# (>= vs >) and the leading negation differ per system and are load-bearing:
+# CW/Geo negate the wrapped value, so phi=180 maps consistently to -180 across
+# all four systems. The dict keys also serve as the whitelist of source systems.
+_TO_ERICSSON = {
+    "SPCS_Polar": lambda t, p: (t, np.where(p >= 180, p - 360, p)),
+    "SPCS_CW": lambda t, p: (t + 90, -np.where(p > 180, p - 360, p)),
+    "SPCS_CCW": lambda t, p: (t + 90, np.where(p >= 180, p - 360, p)),
+    "SPCS_Geo": lambda t, p: (np.flip(t), -np.where(p > 180, p - 360, p)),
+}
+
 
 class AntennaPattern:
     """Antenna pattern class to read, calculate and visualize JSON antenna pattern data.
@@ -78,7 +90,21 @@ def __init__(self, data_filepath: str, validate: bool = False):
         self.raw_data: dict[str, Any] = self._normalize_json(
             self._load_data_from_file(data_filepath)
         )
-        if validate and self._schema is not None:
+        if not self.raw_data.get("Data_Set"):
+            logger.error(
+                f"AntennaPattern: 'Data_Set' is empty or missing in {data_filepath}."
+            )
+            raise ValueError(
+                f"AntennaPattern: 'Data_Set' is empty or missing in {data_filepath}."
+            )
+        if validate:
+            if self._schema is None:
+                logger.error(
+                    "AntennaPattern: Validation requested but no schema is available."
+                )
+                raise ValueError(
+                    "AntennaPattern: Validation requested but no schema is available."
+                )
             self._validate_data_against_schema(self.raw_data, self._schema)
 
         # ---- Process the pattern data into one normalized format ----
@@ -142,12 +168,7 @@ def _validate_data_against_schema(
             error_path_str = (
                 " -> ".join(map(str, e.path)) if e.path else "document root"
             )
-            full_error_message = (
-                f"Antenna data validation FAILED for '{self.data_filepath}'.\n"
-                f"Schema source: '{NGMNSchema.source_message}'.\n"
-                f"Error at data path: '{error_path_str}'.\n"
-                f"Validation Message: {e.message} (Validator: '{e.validator}')"
-            )
+            full_error_message = f"Antenna data validation FAILED for '{self.data_filepath}'.\nSchema source: '{NGMNSchema.source_message}'.\nError at data path: '{error_path_str}'.\nValidation Message: {e.message} (Validator: '{e.validator}')"
             logger.error(full_error_message)
             raise ValidationError(full_error_message) from e
 
@@ -462,7 +483,7 @@ def _process_pattern_data(self) -> xr.Dataset:
         )
 
         # coordinate system and grid
-        if self.coordinate_system not in DEFAULT_INTERNAL_COORD_SYSTEM:
+        if self.coordinate_system != DEFAULT_INTERNAL_COORD_SYSTEM:
             logger.warning(
                 f"AntennaPattern: Coordinate system {self.coordinate_system} not used for calculations. Transforming 'Pattern_3D' attribute to {DEFAULT_INTERNAL_COORD_SYSTEM}."
             )
@@ -498,32 +519,38 @@ def _change_coordinate_system(
             raise NotImplementedError(
                 f"Antenna Pattern: Change to coordinate system {to_system} not implemented yet. Use the default (SPCS_Ericsson) for now."
             )
+        transformable_systems = tuple(_TO_ERICSSON)
+        if from_system not in _TO_ERICSSON:
+            logger.error(
+                f"AntennaPattern: Unsupported source coordinate system '{from_system}'. Expected one of {transformable_systems}."
+            )
+            raise ValueError(
+                f"AntennaPattern: Unsupported source coordinate system '{from_system}'. Expected one of {transformable_systems}."
+            )
         phi = Pattern_3D.coords["Phi"].values
         theta = Pattern_3D.coords["Theta"].values
-        if from_system == "SPCS_Polar":
-            if to_system == "SPCS_Ericsson":
-                Pattern_3D = Pattern_3D.assign_coords(
-                    Theta=("Theta", theta),
-                    Phi=("Phi", np.where(phi >= 180, phi - 360, phi)),
-                )
-        if from_system == "SPCS_CW":
-            if to_system == "SPCS_Ericsson":
-                Pattern_3D = Pattern_3D.assign_coords(
-                    Theta=("Theta", theta + 90),
-                    Phi=("Phi", -np.where(phi > 180, phi - 360, phi)),
-                )
-        if from_system == "SPCS_CCW":
-            if to_system == "SPCS_Ericsson":
-                Pattern_3D = Pattern_3D.assign_coords(
-                    Theta=("Theta", theta + 90),
-                    Phi=("Phi", np.where(phi >= 180, phi - 360, phi)),
-                )
-        if from_system == "SPCS_Geo":
-            if to_system == "SPCS_Ericsson":
-                Pattern_3D = Pattern_3D.assign_coords(
-                    Theta=("Theta", np.flip(theta)),
-                    Phi=("Phi", -np.where(phi > 180, phi - 360, phi)),
-                )
+        # to_system is guaranteed SPCS_Ericsson by the guard above.
+        new_theta, new_phi = _TO_ERICSSON[from_system](theta, phi)
+        Pattern_3D = Pattern_3D.assign_coords(
+            Theta=("Theta", new_theta),
+            Phi=("Phi", new_phi),
+        )
+        new_theta = Pattern_3D.coords["Theta"].values
+        if new_theta.min() < 0 or new_theta.max() > 180:
+            logger.error(
+                f"AntennaPattern: Transformed theta out of range [0, 180] ([{new_theta.min()}, {new_theta.max()}]) converting from '{from_system}'. Input data is likely out of spec for that system."
+            )
+            raise ValueError(
+                f"AntennaPattern: Transformed theta out of range [0, 180] ([{new_theta.min()}, {new_theta.max()}]) converting from '{from_system}'. Input data is likely out of spec for that system."
+            )
+        new_phi = Pattern_3D.coords["Phi"].values
+        if new_phi.min() < -180 or new_phi.max() > 179:
+            logger.error(
+                f"AntennaPattern: Transformed phi out of range [-180, 179] ([{new_phi.min()}, {new_phi.max()}]) converting from '{from_system}'. Input data is likely out of spec for that system."
+            )
+            raise ValueError(
+                f"AntennaPattern: Transformed phi out of range [-180, 179] ([{new_phi.min()}, {new_phi.max()}]) converting from '{from_system}'. Input data is likely out of spec for that system."
+            )
         Pattern_3D = Pattern_3D.assign_attrs(
             coordinate_system=to_system,
         )
@@ -700,6 +727,13 @@ def calculate_beam_efficiency(
 
         weighted_field_values = self.Pattern_3D["dOmega"] * field_values
         Sp_overall = float(weighted_field_values.sum())
+        if Sp_overall == 0:
+            logger.error(
+                "AntennaPattern: Overall power is zero; cannot compute beam efficiency."
+            )
+            raise ValueError(
+                "AntennaPattern: Overall power is zero; cannot compute beam efficiency."
+            )
 
         operators_dict = {
             "<": operator.lt,
@@ -814,19 +848,37 @@ def calculate_top_3db_point(self, power: bool = False) -> float:
             vertical_cut_normed = vertical_cut["P_tp_dB"]
         else:
             vertical_cut_normed = vertical_cut["P_co_dB"]
+        # Fallback: if no point at/below -3 dB exists above the peak (e.g. a very
+        # narrow beam peaking at the top of the cut), the 3 dB border collapses to
+        # the peak theta itself instead of leaving ``top_border`` unbound.
+        top_border = float(theta_val_peak)
+        # Advance the border by the actual theta grid step rather than a hardcoded
+        # 1 deg, so the result is correct for any sampling resolution.
+        theta_axis = np.sort(vertical_cut_normed["Theta"].values)
+        if theta_axis.size > 1:
+            grid_step = float(np.median(np.diff(theta_axis)))
+        else:
+            grid_step = 1.0
         for theta_val in np.flip(
             vertical_cut_normed.sel(Theta=slice(0, theta_val_peak))["Theta"]
         ):
             if vertical_cut_normed.sel(Theta=theta_val) <= -3:
-                top_border = float((theta_val + 1).values)
+                top_border = float(theta_val.values) + grid_step
                 break
+        else:
+            logger.warning(
+                "AntennaPattern: No -3 dB crossing found above the peak; using peak theta as top 3 dB border."
+            )
 
         # enrich with top_3db_point
         self.Pattern_3D.attrs["top_3db_point"] = top_border
         return top_border
 
     def plot(
-        self, component_name: str = "P_tp_dB", show_fig: bool = True, remove_layout_components: bool = False
+        self,
+        component_name: str = "P_tp_dB",
+        show_fig: bool = True,
+        remove_layout_components: bool = False,
     ) -> None | go.Figure:
         """Plots the radiation pattern as heatmap.
 
@@ -860,13 +912,8 @@ def plot(
                 zmin=-30,
                 zmax=0,
                 colorbar={"title": component_name, "thickness": 9},
-                hovertemplate=(
-                    "φ = %{x:.0f}°<br>"
-                    "θ = %{y:.0f}°<br>"
-                    "val = %{z:.2f}<br>"
-                    "<extra></extra>"
-                ),
-                showscale=not(remove_layout_components),
+                hovertemplate="φ = %{x:.0f}°<br>θ = %{y:.0f}°<br>val = %{z:.2f}<br><extra></extra>",
+                showscale=not (remove_layout_components),
             )
         )
         fig.update_yaxes(autorange="reversed")
@@ -884,7 +931,7 @@ def plot(
                     "zeroline": False,
                     "showline": False,
                 },
-                plot_bgcolor="rgba(0,0,0,0)",   # sin fondo gris en el área del heatmap
+                plot_bgcolor="rgba(0,0,0,0)",  # sin fondo gris en el área del heatmap
                 paper_bgcolor="rgba(0,0,0,0)",  # sin fondo/gris alrededor
                 margin={"t": 0, "l": 0, "r": 0, "b": 0},  # recorta al mínimo
                 height=500,
@@ -1077,11 +1124,11 @@ def __str__(self) -> str:
             "==== Parameters ====",
             f"  Gain [dbi]: {self.gain_dbi if self.gain_dbi is not None else 'N/A'}",
             f"  EIRP [dBm]: {self.eirp_dbm if self.eirp_dbm is not None else 'N/A'}",
-            f"  Phi HPBW [deg]: {self.phi_hpbw if self.phi_hpbw else 'N/A'}",
-            f"  Theta HPBW [deg]: {self.theta_hpbw if self.theta_hpbw else 'N/A'}",
-            f"  Front to Back [db]: {self.front_to_back if self.front_to_back else 'N/A'}",
+            f"  Phi HPBW [deg]: {self.phi_hpbw if self.phi_hpbw is not None else 'N/A'}",
+            f"  Theta HPBW [deg]: {self.theta_hpbw if self.theta_hpbw is not None else 'N/A'}",
+            f"  Front to Back [db]: {self.front_to_back if self.front_to_back is not None else 'N/A'}",
             "==== Frequency & Tilt ====",
-            f"  Frequency [Hz]: {self.frequency_hz if self.frequency_hz else 'N/A'}",
+            f"  Frequency [Hz]: {self.frequency_hz if self.frequency_hz is not None else 'N/A'}",
             f"  Frequency Range [Hz]: {self.frequency_range if self.frequency_range is not None else 'N/A'}",
             f"  Theta Electrical Tilt [deg]: {self.theta_eletrical_tilt if self.theta_eletrical_tilt is not None else 'N/A'}",
             f"  Phi Electrical Pan [deg]: {self.phi_eletrical_pan if self.phi_eletrical_pan is not None else 'N/A'}",
@@ -1093,7 +1140,4 @@ def __str__(self) -> str:
         return "\n".join(lines)
 
     def __repr__(self) -> str:
-        return (
-            f"<AntennaPattern(data_filepath='{self.data_filepath}', "
-            f"model='{self.antenna_model}', supplier='{self.supplier}')>"
-        )
+        return f"<AntennaPattern(data_filepath='{self.data_filepath}', model='{self.antenna_model}', supplier='{self.supplier}')>"

src/eas_3d_pattern/sample_data/__init__.py

@@ -7,32 +7,28 @@
 SAMPLE_JSON: list[Path] = []
 
 try:
-    resource_names = list(importlib.resources.contents(__package__))
-    resource_names.sort()
+    anchor = importlib.resources.files(__package__)
+    resources = sorted(anchor.iterdir(), key=lambda res: res.name)
 
-    for item_name in resource_names:
-        if item_name.endswith(".json") and importlib.resources.is_resource(
-            __package__, item_name
-        ):
+    for resource in resources:
+        if resource.name.endswith(".json") and resource.is_file():
             try:
-                with importlib.resources.path(__package__, item_name) as path_context:
+                with importlib.resources.as_file(resource) as path_context:
                     resolved_path = Path(path_context)
 
                 if resolved_path.is_file():
                     SAMPLE_JSON.append(resolved_path)
                 else:
                     logger.warning(
-                        f"Path for sample '{item_name}' in '{__package__}' "
-                        f" ('{resolved_path}') was not a file after context. Skipping."
+                        f"Path for sample '{resource.name}' in '{__package__}'  ('{resolved_path}') was not a file after context. Skipping."
                     )
             except FileNotFoundError:
                 logger.warning(
-                    f"Sample file '{item_name}' listed but not found by "
-                    f"importlib.resources.path in '{__package__}'. Skipping."
+                    f"Sample file '{resource.name}' listed but not found by importlib.resources in '{__package__}'. Skipping."
                 )
             except Exception as e_path:
                 logger.error(
-                    f"Error resolving path for sample '{item_name}' in '{__package__}': {e_path}"
+                    f"Error resolving path for sample '{resource.name}' in '{__package__}': {e_path}"
                 )
 
     if SAMPLE_JSON:
@@ -41,9 +37,7 @@
         )
     else:
         logger.debug(
-            f"No .json sample files found or resolved in '{__package__}'. "
-            "Ensure files exist, have .json extension, and 'include' "
-            "in pyproject.toml is correct for 'sample_data/*.json'."
+            f"No .json sample files found or resolved in '{__package__}'. Ensure files exist, have .json extension, and 'include' in pyproject.toml is correct for 'sample_data/*.json'."
         )
 
 except ModuleNotFoundError:

src/eas_3d_pattern/schema_manager.py

@@ -160,9 +160,11 @@ def _load_bundled(self) -> dict[str, Any]:
             f"SchemaManager: Attempting to load bundled schema: {self.bundled_package_ref}/{self.bundled_filename}"
         )
         try:
-            with importlib.resources.open_text(
-                self.bundled_package_ref, self.bundled_filename
-            ) as sf:
+            schema_resource = (
+                importlib.resources.files(self.bundled_package_ref)
+                / self.bundled_filename
+            )
+            with schema_resource.open(encoding="utf-8") as sf:
                 content = json.load(sf)
             self.source_message = (
                 f"Bundled Schema ({self.bundled_package_ref}/{self.bundled_filename})"
@@ -205,11 +207,7 @@ def _load_and_validate_schema(self):
             ) from e
 
     def __repr__(self) -> str:
-        return (
-            f"<{self.__class__.__name__}(id=0x{id(self):x}, "
-            f"schema_loaded={'True' if self.schema_content else 'False'}, "
-            f"source={self.source_message})>"
-        )
+        return f"<{self.__class__.__name__}(id=0x{id(self):x}, schema_loaded={'True' if self.schema_content else 'False'}, source={self.source_message})>"
 
     def __str__(self) -> str:
         return str(self.schema_content)

src/eas_3d_pattern/sector_definitions.py

@@ -59,11 +59,7 @@ def __post_init__(self):
             )
 
     def __str__(self):
-        return (
-            f"'{self.name}': \t"
-            f"[{self.theta_min[0]:.1f}{self.theta_min[1]}Theta{self.theta_max[1]}{self.theta_max[0]:.1f}], "
-            f"[{self.phi_min[0]:.1f}{self.phi_min[1]}Phi{self.phi_max[1]}{self.phi_max[0]:.1f}]"
-        )
+        return f"'{self.name}': \t[{self.theta_min[0]:.1f}{self.theta_min[1]}Theta{self.theta_max[1]}{self.theta_max[0]:.1f}], [{self.phi_min[0]:.1f}{self.phi_min[1]}Phi{self.phi_max[1]}{self.phi_max[0]:.1f}]"
 
 
 class SectorDefinition: