Repository navigation
Refactor/split parser phase2 - #14
Merged
Merged
Conversation
Move 21 pure read-only metadata properties out of the AntennaPattern god class into a new eas_3d_pattern.ngmn package (ngmn/metadata.py::Metadata), which AntennaPattern now inherits. Property bodies are moved verbatim; behaviour and public API are unchanged. The ngmn package is future-proofing: further NGMN-bound concerns (schema, coordinates) can be added as sibling modules without renames. Concise Google-style docstrings added so the new module is D102-clean without spreading parser.py's per-file suppression (aligns with plan M2-5). Add TestPassThroughMetadata (14 tests) to pin the value contract of the moved accessors: BASTA_AA_WP_version and coordinate_system previously had no coverage, and several others were only executed via __str__ without asserting their return value. Introduce the agreed SPDX/author header on new files (no retrofit). Add project-local .trash/ to .gitignore for refactor backups. parser.py: 1049 -> 966 lines. Phase 2a of the god-class decomposition (plan section 2.1.3). Unit-conversion properties (2b) and sampling/preset properties (2c) remain for follow-up milestones. Verified: ruff check, ruff format, mypy (11 files), pytest (144 passed).
…e is_nonuniform_sampling (Phase 2c) Move theta_sampling, phi_sampling, raw_pattern_dataframe, is_uniform_sampling and is_nonuniform_sampling from AntennaPattern into ngmn/metadata.py::Metadata, completing the metadata extraction (plan section 2.1.3, Phase 2). is_nonuniform_sampling now emits a DeprecationWarning and delegates to 'not is_uniform_sampling'; it is redundant and scheduled for removal in a future release (plan section 2.9). Recorded under Deprecated in CHANGELOG. Extract the magic 1e-6 sampling offset into a named _SAMPLING_STOP_EPSILON constant in the new module and drop the now-unused module-level 'epsilon' from parser.py (chips at plan section 2.8). Add the pandas import and Google-style docstrings in the new module. sector_preset / available_sector_presets stay on AntennaPattern: they touch instance state and SectorDefinition, so they are configuration, not NGMN metadata reads. Add TestSamplingMetadata (5 tests): grid shapes, dataframe columns, the uniform flag, and the deprecation warning. The non-uniform/absent-triple branch of the sampling grids is not reachable through the current fixture and is left for a future non-uniform fixture.
…ase 3a) Move ensure_domega, directivity and losses out of AntennaPattern into a new eas_3d_pattern.metrics package: metrics/quadrature.py holds the shared solid-angle weights, metrics/directivity.py the two figures of merit. Implemented as pure functions over the pattern dataset, following the _plotting.py precedent rather than a mixin: they take pattern_3d and return a number, so they can be used and tested without constructing an AntennaPattern. AntennaPattern keeps thin wrappers, so the public API and the current verbose method names are unchanged (renaming is M2-14). quadrature.py is separate from directivity.py because ensure_domega is shared with the beam-efficiency metric arriving in Phase 3c; keeping it standalone avoids efficiency.py importing from directivity.py. The deliberate in-place dOmega cache is preserved, as is the short-circuit order in losses(): gain is validated before directivity is computed. The repeated "dOmega" literal is now a DOMEGA constant (plan section 2.8); the data-variable name on the dataset is unchanged. Drops the "Delaunay+Voronoi is not supported" caveat from the directivity docstring — plan section 1.3.1 measured this and found no accuracy limitation for conforming NGMN data, so the note was misleading. metrics/ is intentionally not re-exported from the top-level package, so no second public API surface is committed to while M2-14 is open. No new tests: test_directivity.py already covers this code at 100%. parser.py: 829 -> 790 lines. Verified: ruff check, ruff format, mypy (14 files), pytest (149 passed), and directivity / sum(dOmega) / Cell efficiency / peak / top-3dB / losses are byte-identical to the pre-change capture on both bundled samples.
Move the beam-peak search and the top 3 dB border out of AntennaPattern into
metrics/peak.py as pure functions: find_peak(pattern_3d, power) and
top_3db_border(pattern_3d, theta_peak, phi_peak, power).
top_3db_border takes the peak as an argument instead of locating it, so the
AntennaPattern wrappers keep ownership of the dataset side effects.
calculate_top_3db_point still goes through self.find_peak_coordinates(), which
is what publishes peak_coordinates — util_func/report.py calls only that method
and then reads the attribute, so the transitive side effect is load-bearing.
Extract the "P_tp_dB" / "P_co_dB" component names, the -3 dB half-power level
and the fallback grid step into named constants, and collapse the duplicated
component-selection branch into a single component_name() helper (plan
section 2.8).
Tests first, per plan section 2.1.3: tests/test_peak.py (8 tests) pins the
previously uncovered power=True paths of both methods, the single-theta-point
grid-step fallback, and both published attrs. They were written and passing
against the unmoved code before the extraction.
The non-tuple guard in find_peak is unreachable and stays uncovered:
stack(pt=("Theta","Phi")) raises KeyError when a dimension is missing, and
with both present the stacked coordinate is always a tuple. Recorded as a
comment in the test file, pending a decision on removing it as dead code.
parser.py: 790 -> 758 lines. metrics/peak.py at 94% (only the dead guard).
Verified: ruff check, ruff format, mypy (15 files), pytest (157 passed), and
the directivity / dOmega / efficiency / peak / top-3dB canary is identical to
the pre-refactor capture on both bundled samples.
…c link (Phase 3c) Move the beam-efficiency integration out of AntennaPattern into metrics/efficiency.py as a pure function beam_efficiency(pattern_3d, sectors, powersum), with the per-sector masking factored into a _sector_sum helper. Preset dispatch (_build_sectors_from_preset) deliberately stays on AntennaPattern: it calls calculate_top_3db_point(), so beam efficiency currently publishes peak_coordinates and top_3db_point on the dataset as a side effect. Threading that through a pure dispatcher would change which attributes get published, so orchestration stays with the class and only the numeric integration moves. This narrows the plan's Phase 3c scope. Remove the now-dead private _ensure_domega wrapper: after this phase nothing calls it, since directivity() and beam_efficiency() invoke ensure_domega() directly. The in-place dOmega mutation itself is unchanged (plan 2.1.2). Extract the linear component names into constants and the boundary comparison operators into a read-only MappingProxyType lookup, replacing a dict rebuilt on every call (plan section 2.8). Remove the internal Ericsson eridoc link from the calculate_beam_efficiency docstring, per owner approval. src/ is now free of internal document links (plan section 12.1); the Example_02 notebook occurrence remains open. Tests first, per plan section 2.1.3: tests/test_efficiency.py (8 tests) pins the default eas preset path, the Type A missing-HPBW guard, the externally registered preset fallback, powersum=False, and the sector type guard. All five were uncovered before; parser.py went 85% -> 88% on the tests alone. parser.py: 758 -> 675 lines. metrics/ at 100% except the documented unreachable guard in peak.py. Verified: ruff check, ruff format, mypy (16 files), pytest (165 passed), and the directivity / dOmega / efficiency / peak / top-3dB canary is identical to the pre-refactor capture on both bundled samples.
_process_pattern_data selected the non-uniform branch via is_nonuniform_sampling, which Phase 2c deprecated. Loading any pattern without Theta_Sampling / Phi_Sampling triples therefore raised a DeprecationWarning naming an API the caller never used. Verified against the bundled non-uniform sample: one warning before, none after. Replaced with 'not self.is_uniform_sampling', which is exactly what the deprecated property returns, so behaviour is otherwise unchanged. The synthetic test fixture only ever produced uniform sampling, so no test exercised the branch. Add build_nonuniform_pattern_dict and the nonuniform_pattern_path fixture, which enumerate Theta/Phi per data row and omit the sampling triples — the layout real NGMN NonUniformSampling files use (plan section 1.3.1). tests/test_nonuniform_loading.py (5 tests) covers the non-uniform load path, asserts no self-inflicted DeprecationWarning on either load path, closes the theta_sampling / phi_sampling None branches left uncovered in Phase 2c, and cross-checks that the same beam declared either way gives identical directivity and peak so the two branches cannot drift. Verified: ruff check, ruff format, mypy (16 files), pytest (170 passed), canary identical. ngmn/metadata.py coverage 92% -> 93%.
Move _change_coordinate_system out of AntennaPattern into ngmn/coordinates.py as to_internal_frame(pattern_3d, from_system, to_system), together with the _TO_ERICSSON transform table, DEFAULT_INTERNAL_COORD_SYSTEM and EXPECTED_COORDINATE_SYSTEMS. The method was private with a single internal caller, so no wrapper is kept; _process_pattern_data calls the function. Factor the two post-condition guards into _reject_out_of_range and replace the repeated [0, 180] / [-180, 179] literals with THETA_RANGE_DEG / PHI_RANGE_DEG constants (plan section 2.8). The rendered error messages are unchanged, which the existing out-of-range tests confirm. EXPECTED_COORDINATE_SYSTEMS is unreferenced anywhere in src/ or tests/ — the real source whitelist is the _TO_ERICSSON keys. Moved rather than deleted, since it documents the NGMN-named systems and is a public module-level name, and marked informational-only. Removal is still open. Tests first, per plan section 2.1.3: the NotImplementedError guard for a non-Ericsson target was written against the original method and confirmed passing before the move, then retargeted at the function, plus a new test for the unsupported-source guard. parser.py: 675 -> 603 lines. ngmn/coordinates.py at 100% coverage. Verified: ruff check, ruff format, mypy (17 files), pytest (172 passed), canary identical on both bundled samples.
… 4b) Move _load_data_from_file, _normalize_json and _validate_data_against_schema out of AntennaPattern into ngmn/loader.py as load_json_file, normalize_keys and validate_against_schema, together with the ALTERNATIVES key map. validate_against_schema takes data_filepath and schema_source as arguments instead of reaching for the NGMNSchema singleton, so the loader carries no dependency on it and will not need changing when the schema manager is reworked to load lazily (plan section 1.2). test_normalize.py previously invoked the method unbound as _normalize_json(None, data) with a type: ignore; it now calls the function directly, dropping both the fake self and the suppression. Tests first, per plan section 2.1.3: tests/test_loader.py (10 tests) pins the failure paths that were the least covered code in the parser — missing file, directory instead of file, malformed and empty JSON, validation failure and its enriched message, and missing Data_Set. Written and passing against the unmoved code, taking parser.py from 86% to 95% before anything moved. Also adds a __repr__ test and a slow-marked test asserting a bundled NGMN sample validates against the shipped schema — the only route to the validation success path, and a guard against sample and schema drifting apart. Registers the slow marker in pyproject.toml. parser.py: 603 -> 538 lines, 95% covered. ngmn/loader.py at 100%. Verified: ruff check, ruff format, mypy (18 files), pytest (182 passed), canary identical on both bundled samples.
Move _process_pattern_data out of AntennaPattern into _processing.py as PatternProcessing, which extends ngmn.metadata.Metadata and is inherited by AntennaPattern. The method body is unchanged. Decomposing it is deliberately deferred and recorded as a todo in the module docstring: it still does five separable jobs (sampling-format branch and grid assembly, field-component derivation, dataset construction, coordinate transform dispatch, grid-irregularity reporting), only the first of which is NGMN-specific and only the second of which is antenna mathematics, so the module straddles the ngmn / metrics boundary drawn elsewhere. Fix two calls that used the root logger via logging.error() instead of the module logger. That bypassed the NullHandler the package installs on its own logger, so those messages could reach stderr in a host application that never configured logging. Extract the component column list to COMPONENT_COLUMNS. Tests first, per plan section 2.1.3: tests/test_processing.py (7 tests) pins the sampling/row-count mismatch and its diagnostic counts, duplicate (Theta, Phi) rejection, and the irregular-grid warnings, taking parser.py from 95% to 99% before the move. The "No uniform or nonuniform sampling detected" branch is confirmed unreachable: is_uniform_sampling is bool(Theta_Sampling and Phi_Sampling) and theta_sampling returns None under that same falsy condition, so a true is_uniform_sampling guarantees both accessors are present and the elif absorbs the rest. TestUnreachableSamplingBranch asserts that invariant instead of fabricating the state. It is the only uncovered code in _processing.py. parser.py: 538 -> 422 lines, now at 100% coverage. Verified: ruff check, ruff format, mypy (19 files), pytest (189 passed), canary identical on both bundled samples.
Metadata (ngmn/metadata.py):
- Align required-field accessors with the NGMN BASTA JSON schema: the nine
non-nullable required string properties (supplier, antenna_model,
antenna_type, revision_version, released_date, coordinate_system,
pattern_type, nominal_polarization, BASTA_AA_WP_version) stay `-> str` and
raise KeyError when absent, with docstrings stating the contract.
- Fix optional_comments: it is not a schema field, so it is now genuinely
optional (`-> str | None` via .get()) instead of a hard subscript that
would raise on absence.
Schema manager (schema_manager.py):
- Add SchemaSource enum (URL, CACHE, BUNDLED) modelling schema provenance;
the human-readable rendering lives on the enum via SchemaSource.message().
schema_source replaces the ad-hoc source_message string, which is now a
derived property. Cache behaviour is unchanged (CACHE not yet reachable).
- Make the JSON Schema draft configurable via SCHEMA_VERSION / self.schema_version
instead of a hardcoded validator class. After load, cross-check the schema's
declared "$schema" dialect against the configured version and raise if it
does not match ("not configured for that version"); select the validator via
a match with a default arm that raises on unsupported versions.
- Add SchemaManager.validate(data, data_filepath): a thin adapter that supplies
this manager's own schema and source to the pure loader mechanism and owns the
"no schema available" guard.
Parser (parser.py):
- __init__ now accepts str | Path for data_filepath and stores it as a Path;
normalize -> validate -> commit ordering. Replaces os.path with pathlib and
drops the now-unused os import.
- Validation collapses to NGMNSchema.validate(self.raw_data, self.data_filepath);
the parser no longer inspects NGMNSchema internals.
- Rework __str__: drop the dead 'N/A' guard from the guaranteed-str required
fields and route every genuinely optional field through a single _na() helper
(is-None based, fixing the falsy-zero display bug).
API: export SchemaSource from the package.
Tests:
- Add tests/test_schema_manager.py covering the SchemaSource enum and its
message(), manager provenance state, version cross-check (match/mismatch/
missing dialect) and validator selection (supported/unsupported).
Docs:
- CHANGELOG updated; notebooks updated.
BREAKING CHANGE: AntennaPattern.data_filepath is now a pathlib.Path (was the
raw str as passed). The nine required metadata properties raise KeyError when
their key is absent rather than returning a value; a loaded schema whose
"$schema" dialect does not match the configured SCHEMA_VERSION is now rejected.
…deprecation aliases
- ngmn.loader: split json_load (load + normalize) from pure load_json_file; export json_load
- util_func/guards.py: extract reusable verify() check-and-raise helper from AntennaPattern
- rename AntennaPattern.raw_data -> data (payload is normalized, not raw); keep raw_data as
deprecated read-only alias that warns
- rename AntennaPattern.Pattern_3D -> pattern (PEP 8, drop redundant _3D); keep Pattern_3D as
deprecated read-only alias that warns
- de-privatize modules: _plotting.py -> plotting.py, _processing.py -> processing.py
- update tests, notebooks, CHANGELOG accordingly
All gates green: ruff, mypy (20 files), 209 tests pass.
Break the monolithic _process_pattern_data into an orchestrator over five
named helpers, no behavioural change:
- _get_pattern_from_uniform_sampling: uniform-grid meshgrid assembly, with
verify()-guarded sampling-vector invariants (staticmethod, typed)
- _derive_field_components: dB->linear, phase->rad, complex E-fields, and the
TP present/reconstruct branch (staticmethod)
- _build_dataset: to_xarray + metadata attrs (local renamed df -> ds)
- _apply_coordinate_transform: internal-frame dispatch
- _warn_on_irregular_grid: single axis loop over Theta/Phi (was a duplicated
double-if; also fixes the 'unfirom' typo and uses lazy %s logging)
Also:
- replace the missing-column for-loop with pd.Index.difference set subtraction
- replace phase all-NaN zeroing with fillna(0) (equivalent for schema-valid
input: Data_Set cells are non-null, so a phase column is all-or-nothing)
- add tests/test_guards.py covering the verify() contract
- rename test class to TestUniformSamplingInvariant and refresh its docstring
- resolve the module .. todo:: now that the decomposition is done; fix a
docstring grammar slip
All gates green: ruff, mypy (20 files), 228 tests pass.
Replace the single sector_definitions.py module with a sector/ package that
separates geometry, the collection, and presets:
- sector.definitions: BoundaryBox (renamed from BoundaryBoxSquare, 'Square'
dropped; name field removed — a box is pure geometry, the Sector names it via
its dict key) and Sector, the named box collection. Preset-agnostic, so it has
no import cycle with the presets.
- sector.presets: SectorPreset ABC with a load() -> Sector method, plus the
registry helpers from_preset / preset_names / validate_preset.
- sector.eas_preset.EasPreset and sector.ngmn_type_a_preset.NgmnTypeAPreset:
one preset per module, each owning its constants; presets are now classes
rather than free builder functions.
- sector._compat.SectorDefinition: backward-compatible Sector subclass keeping
the deprecated load_default/top_border constructor and the from_preset/
validate_preset/presets classmethods, all delegating to sector.presets.
Backward compatibility preserved: SectorDefinition and BoundaryBoxSquare remain
importable from eas_3d_pattern unchanged; BoundaryBoxSquare is now an alias of
BoundaryBox. parser, report and metrics.efficiency type against the Sector base.
CHANGELOG updated (Changed: package move + class-based presets; Deprecated:
BoundaryBoxSquare alias). trashcan/ added to .gitignore.
All gates green: ruff, mypy (25 files), 228 tests pass.
Replace two ad-hoc 'if ...: raise ValueError' checks with the shared verify()
guard, matching the pattern used across the codebase:
- metrics/directivity.py: losses() now guards the missing-gain case with
verify(gain_dbi is not None, ...).
- parser.py: the NGMN Type A preset splits the combined Theta/Phi HPBW check
into two per-field verify() calls, so the error names exactly which field is
missing. Also drops a redundant explicit power=False (it is the default).
Both add a cast() after the guard, since verify() does not narrow Optional for
mypy the way an inline if-raise does.
All gates green: ruff, mypy (25 files), 228 tests pass.
- Replace the zero-power 'if ...: log + raise' with verify(Sp_overall != 0, ...).
- Replace the per-sector isinstance check with
verify(isinstance(box, BoundaryBox), ..., TypeError), migrating off the
deprecated BoundaryBoxSquare alias to the standard BoundaryBox name
(import, type annotations, docstrings, and the test match updated to suit).
- Rename the internal beam_efficiency parameter pattern_3d -> pattern for
consistency (local only, no caller impact).
All gates green: ruff, mypy (25 files), 228 tests pass.
- Replace the 'if not isinstance(peak_tuple, tuple): log + raise' with
verify(isinstance(peak_tuple, tuple), ...).
- Rename the local parameter pattern_3d -> pattern in find_peak and
top_3db_border for consistency (signatures and docstrings), no caller impact.
All gates green: ruff, mypy (25 files), 228 tests pass.
coordinates.py:
- Convert all four check-and-raise sites to verify(): the unsupported target
system (NotImplementedError), the unsupported source system (ValueError), and
the two post-transform theta/phi out-of-range checks (ValueError).
metadata.py:
- Extract the duplicated frequency unit->Hz logic into a module-level
_HZ_MULTIPLIER table + _to_hz() helper; frequency_hz and frequency_range both
delegate (kept as separate properties — distinct fields and return contracts).
- Extract the power conversions into module-level _to_dbm() and _to_watt()
helpers; eirp_dbm and output_power_watt delegate. Two functions rather than one
table, since the target units (dBm vs W) use different formulas.
report.py:
- Drop a decorative emoji from a log message.
Behaviour preserved (unknown-unit fall-through kept); all gates green: ruff,
mypy (25 files), 228 tests pass.
- Correct stale module names (processing.py/plotting.py, de-privatized) and the
test count (189 -> 228).
- Add entries for the new verify() guard helper (util_func.guards) and json_load()
in ngmn.loader.
- Add before/after code examples to the SectorDefinition deprecation entry showing
the deprecated constructor, the new from_preset()/Sector() way, and the still-
supported SectorDefinition.from_preset() classmethod (all examples verified).
The verify() guard conversions and the frequency/power unit-converter extractions
are internal refactors with no API or behaviour change, so they carry no changelog
entry; only new public-facing names are listed.
… point
DeprecationWarning is silenced by default, so end users - who are not
expected to run test suites - never saw the deprecation notices. Switch
all deprecated entry points to FutureWarning and add a logger.warning on
the eas_3d_pattern logger.
- raw_data, Pattern_3D, is_nonuniform_sampling: FutureWarning + logger
- SectorDefinition: warn unconditionally in __init__ (previously only when
load_default was true, so the documented migration target itself was
silent) and in presets/validate_preset/from_preset, redirecting to the
module-level eas_3d_pattern.sector helpers
- BoundaryBoxSquare: replace the silent class alias with a BoundaryBox
subclass that warns on construction; isinstance(x, BoundaryBox) still
holds, equality is now class-sensitive
- add Sphinx .. deprecated:: directives to all deprecated docstrings
- add notebooks/Example_06_Deprecations_and_Migration.ipynb
Example_02 and the showcase built an empty sector collection with
SectorDefinition(load_default=False). That call now emits a
FutureWarning, since the compatibility shim warns on every
construction rather than only when load_default is true.
- replace SectorDefinition(load_default=False) with Sector(); the
following add_sector() calls are unchanged, as add_sector is
defined on Sector
- import Sector from eas_3d_pattern.sector, since neither Sector nor
the preset helpers are re-exported at the package top level, unlike
the deprecated SectorDefinition
- refresh executed cell outputs
Example_06 keeps its deprecated calls on purpose: it is the migration
guide, and each one demonstrates the warning it documents.
The deprecation switch in 4fbe3a7 left five tests asserting DeprecationWarning, which FutureWarning does not subclass, so they failed. Fix those, migrate the suite off the deprecated shim, and promote the category to an error so it cannot regress. - swap the five stale pytest.warns categories to FutureWarning in test_loader.py and test_metadata_properties.py, and correct the two docstrings that still named DeprecationWarning; the match= patterns were unchanged and still hold - migrate fourteen internal shim usages to the canonical API across test_ngmn_sectors.py, test_efficiency.py and test_parser.py: SectorDefinition.from_preset -> from_preset (10), SectorDefinition(load_default=False) -> Sector() (3), SectorDefinition.presets -> preset_names (1). This also gives the module-level helpers their first direct coverage - add TestDeprecatedSectorDefinitionShim so retiring those call sites does not drop coverage of the shim itself: the classmethods still delegate to the same results and warn, and construction warns for both load_default values, pinning the load_default=False branch that used to be silent - widen the three absence-checks in test_nonuniform_loading.py and test_resources.py to DeprecationWarning | FutureWarning. They filtered on DeprecationWarning alone, so after the switch they matched nothing and passed unconditionally, silently stopping the self-inflicted-warning guard. Verified by injecting a FutureWarning into the load path: both now fail, where before they did not - add error::FutureWarning to the pytest filterwarnings, now that no test leaks one; deliberate uses must be wrapped in pytest.warns
…otebook showcasing horizontal and vertical cuts
…smatch between local and remote test environment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
maininto this branchWhat does this PR do?
Major refactor of the codebase with alignment to standard PEP python guidelines.
Code splits into modules and submodules.
Few deprecations which are still fully functional but provide the user with a FutureWarning.