Drop the prefixes the packages say, and merge the impulsive methods - #481
Conversation
Four room modules and the aircraft certification module repeated their package in their own name; two of them said less than the new name does, since 'ir' was opaque and 'noise' did not name the ANSI/ASA S12.2 criteria. NT ACOU 112 and ISO/PAS 1996-3 are one module now. They share the prominence and adjustment formulae and differ only in what the caller supplies, and the ISO/PAS module was already importing the Nordtest one for its formulae. The IEC 61265 measurement-system check moves from filters to aircraft, next to the certification levels whose chain it qualifies.
There was a problem hiding this comment.
Sorry @jmrplens, your pull request is larger than the review limit of 150000 diff characters
|
You've reached the fair usage limit for the Bito Code Review Agent trial. Upgrade to a paid plan or contact support@bito.in for assistance. Learn more |
Reviewer's GuideThis PR consolidates the impulsive-sound prominence helpers into the ISO/PAS 1996-3 module, renames several room and aircraft modules to clearer, prefix-free names while preserving backwards-compatible shims, and relocates the IEC 61265 aircraft measurement-system compliance code from filters to the aircraft namespace, updating imports, docs, tests and taxonomy accordingly. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThe pull request reorganizes room and aircraft modules, consolidates impulsive-sound APIs, adds compatibility aliases, and updates internal imports, API documentation, guides, taxonomy data, changelog entries, and tests. ChangesAPI module reorganization
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/api_taxonomy.py`:
- Around line 362-364: Update the comment above the "plot_excitation" entry to
replace the stale "room_ir" reference with the canonical
phonometry.room.impulse_response module name, keeping the override unchanged and
avoiding deprecated pre-4.0 paths.
In `@scripts/generate_graphs.py`:
- Around line 10577-10579: Update the import of ADJUSTMENT_THRESHOLD in the
graph-generation code to use the canonical
phonometry.environment.assessment.impulsive_sound module instead of the
deprecated impulse_prominence alias, leaving the symbol’s usage unchanged.
In `@site/src/content/docs/es/guides/impulse-prominence.mdx`:
- Line 367: Remove the duplicated API-reference list item from the Spanish guide
at site/src/content/docs/es/guides/impulse-prominence.mdx lines 367-367 and from
the English guide at site/src/content/docs/guides/impulse-prominence.mdx lines
358-358; retain the identical entry that follows in each guide.
In `@src/phonometry/aircraft/__init__.py`:
- Around line 112-116: Update the aircraft namespace compatibility setup around
__getattr__ and __dir__ to register the deprecated
phonometry.aircraft.aircraft_noise alias in sys.modules, mapping it to
phonometry.aircraft.certification, so package-level attribute access resolves
correctly. Include the 5.0 removal notice for the old alias path while
preserving the existing namespace shim behavior.
In `@src/phonometry/environment/assessment/impulsive_sound.py`:
- Around line 217-220: Update the prominence input validation around the
onset_rate and level_difference arrays to reject any non-finite values,
including NaN and infinity, before calculation. Also update the rating-level
validation around durations and reference_time to require finite values before
calculation; apply both changes in impulsive_sound.py at lines 217-220 and
343-346.
In `@src/phonometry/filters/__init__.py`:
- Around line 60-62: Restrict the compatibility behavior in the module-level
__getattr__ and __dir__ setup to only expose verify_aircraft_noise_system,
rather than forwarding the complete __all__ from phonometry.aircraft via
_namespace_shim and _namespace_dir. Add an explicit allow-list or name-specific
forwarding hook while preserving compatibility for that moved filter name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 11325ca6-5569-451a-96ba-318c0029ffb3
⛔ Files ignored due to path filters (1)
site/src/generated/api-sidebar.mjsis excluded by!**/generated/**
📒 Files selected for processing (83)
CHANGELOG.mddocs/api-reference.mddocs/room-acoustics.mddocs/room-impulse-response.mddocs/room-noise.mdllms-full.txtllms.txtscripts/api_taxonomy.pyscripts/conformance_report.pyscripts/generate_graphs.pysite/public/llms/llms-room-acoustics.txtsite/src/content/docs/es/guides/aircraft-noise.mdxsite/src/content/docs/es/guides/impulse-prominence.mdxsite/src/content/docs/es/guides/room-acoustics.mdxsite/src/content/docs/es/guides/room-image-sources.mdxsite/src/content/docs/es/guides/room-impulse-response.mdxsite/src/content/docs/es/guides/room-noise.mdxsite/src/content/docs/guides/aircraft-noise.mdxsite/src/content/docs/guides/impulse-prominence.mdxsite/src/content/docs/guides/room-acoustics.mdxsite/src/content/docs/guides/room-image-sources.mdxsite/src/content/docs/guides/room-impulse-response.mdxsite/src/content/docs/guides/room-noise.mdxsite/src/content/docs/guides/system-measurement.mdxsite/src/content/docs/reference/api/aeroacoustics/certification.mdsite/src/content/docs/reference/api/aeroacoustics/measurement-system.mdsite/src/content/docs/reference/api/aeroacoustics/rotorcraft-noise.mdsite/src/content/docs/reference/api/electroacoustics/swept-sine.mdsite/src/content/docs/reference/api/environment/impulse-prominence.mdsite/src/content/docs/reference/api/environment/impulsive-sound.mdsite/src/content/docs/reference/api/filters/compliance.mdsite/src/content/docs/reference/api/index.mdsite/src/content/docs/reference/api/noise_control/duct-path.mdsite/src/content/docs/reference/api/noise_control/room-to-room.mdsite/src/content/docs/reference/api/rooms/acoustics.mdsite/src/content/docs/reference/api/rooms/image-source.mdsite/src/content/docs/reference/api/rooms/impulse-response.mdsite/src/content/docs/reference/api/rooms/modes.mdsite/src/content/docs/reference/api/rooms/noise-criteria.mdsite/src/content/docs/reference/api/signals/inversion.mdsite/src/content/docs/reference/api/signals/phase.mdsrc/phonometry/__init__.pysrc/phonometry/_compat.pysrc/phonometry/_plot/aircraft.pysrc/phonometry/_plot/common.pysrc/phonometry/_plot/environment.pysrc/phonometry/_plot/room.pysrc/phonometry/_report/annex16_epnl.pysrc/phonometry/_report/ansi_s12_2.pysrc/phonometry/_report/iso1996_impulse.pysrc/phonometry/_report/iso3382.pysrc/phonometry/aircraft/__init__.pysrc/phonometry/aircraft/certification.pysrc/phonometry/aircraft/measurement_system.pysrc/phonometry/aircraft/rotorcraft_noise.pysrc/phonometry/electroacoustics/swept_sine.pysrc/phonometry/environment/assessment/__init__.pysrc/phonometry/environment/assessment/impulse_prominence.pysrc/phonometry/environment/assessment/impulsive_sound.pysrc/phonometry/filters/__init__.pysrc/phonometry/filters/compliance.pysrc/phonometry/noise_control/_criterion.pysrc/phonometry/noise_control/duct_path.pysrc/phonometry/noise_control/room_to_room.pysrc/phonometry/room/__init__.pysrc/phonometry/room/acoustics.pysrc/phonometry/room/impulse_response.pysrc/phonometry/room/modes.pysrc/phonometry/room/noise_criteria.pytests/aircraft/test_aircraft_noise_system.pytests/aircraft/test_certification.pytests/aircraft/test_epnl_report.pytests/conftest.pytests/noise_control/test_room_to_room_norton.pytests/room/test_acoustics.pytests/room/test_golay.pytests/room/test_impulse_response.pytests/room/test_modes.pytests/room/test_noise_criteria.pytests/room/test_room_noise_report.pytests/room/test_room_plot_i18n.pytests/test_deprecated_aliases.pytests/test_package_architecture.py
💤 Files with no reviewable changes (4)
- src/phonometry/environment/assessment/impulse_prominence.py
- site/src/content/docs/reference/api/filters/compliance.md
- src/phonometry/filters/compliance.py
- site/src/content/docs/reference/api/environment/impulse-prominence.md
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #481 +/- ##
==========================================
- Coverage 96.99% 96.99% -0.01%
==========================================
Files 273 273
Lines 38617 38664 +47
==========================================
+ Hits 37458 37503 +45
- Misses 1159 1161 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Numerical conformance report✅ 533/533 conformance checks pass across 57 domains and 362 standards - filters class 1 - weightings within IEC 61672-1 class 1. Each row pins a standard clause to its expected normative value and the value the library computes. Every section below is collapsible and stays collapsed while all of its rows pass; a section with any failing row opens automatically. ✅ Numerical validation - filters & weightings: class showcase (IEC 61260-1 · IEC 61672-1 · ISO 7196)IEC 61260-1:2014 class per filter architecture (order 6, one-third-octave, 100 Hz-10 kHz, fs = 48 kHz). For each architecture the table shows, at its binding band, the measured relative attenuation and the class-1 limit it must clear, so the number and the range it must sit in are both visible. A positive margin means the acceptance limits are met with that much room.
Only Butterworth (the library default) and Chebyshev-II are class-compliant architectures. Chebyshev-I and elliptic trade the mask for passband ripple, and Bessel for a maximally-flat group delay (soft rolloff); they cannot satisfy the IEC 61260-1 Class 1/2 attenuation mask by construction, so they are labelled By design - this is expected, not a failure or regression. Frequency-weighting conformance (A/C: IEC 61672-1 Table 3; G: ISO 7196 A.3). The max deviation from nominal is informational (it falls at a frequency extreme where the tolerance is widest and asymmetric); compliance is judged at the binding frequency - the one with the least headroom - where the deviation, the applicable tolerance band and the headroom are shown together.
✅ Filters & weightings: 100% (10/10)
✅ Levels & dosimetry: 100% (9/9)
✅ Room & building acoustics: 100% (72/72)
✅ Room acoustics: 100% (16/16)
✅ Psychoacoustics: 100% (14/14)
✅ Speech transmission (IEC 60268-16): 100% (10/10)
✅ System measurement (Golay / Kirkeby / Mueller-Massarani): 100% (5/5)
✅ Intensity & sound power: 100% (10/10)
✅ Building prediction & uncertainty: 100% (15/15)
✅ Outdoor propagation & occupational exposure: 100% (10/10)
✅ Materials: absorption, airflow & impedance: 100% (6/6)
✅ Scattering & diffusion (ISO 17497): 100% (14/14)
✅ In-situ road absorption (ISO 13472): 100% (3/3)
✅ Precision sound power (ISO 3745 / 9614-3): 100% (4/4)
✅ Human vibration (ISO 8041 / 2631 / 5349): 100% (15/15)
✅ Speech intelligibility (ANSI S3.5-1997): 100% (24/24)
✅ Objective intelligibility (STOI / ESTOI): 100% (3/3)
✅ Impulsive-sound prominence (NT ACOU 112): 100% (2/2)
✅ Impulsive-sound prominence (ISO/PAS 1996-3): 100% (2/2)
✅ Room noise (ANSI S12.2-2019): 100% (3/3)
✅ Hearing threshold (ISO 7029 / ISO 389-7): 100% (3/3)
✅ Measurement uncertainty (GUM / Supplement 1): 100% (7/7)
✅ Noise-induced hearing loss (ISO 1999): 100% (6/6)
✅ Multiple-shock whole-body vibration (ISO 2631-5): 100% (6/6)
✅ Sound absorption in enclosed spaces (EN 12354-6): 100% (2/2)
✅ Prominent discrete tones (ECMA-418-1): 100% (2/2)
✅ Tonal audibility (ISO/PAS 20065): 100% (11/11)
✅ Psychoacoustic annoyance & fluctuation strength (Fastl & Zwicker): 100% (3/3)
✅ Electroacoustics: distortion & frequency response: 100% (20/20)
✅ Calibrated spectral analysis (Bendat & Piersol): 100% (12/12)
✅ Multiple-input coherence (Bendat & Piersol): 100% (5/5)
✅ Time-frequency analysis (Bendat & Piersol): 100% (3/3)
✅ Correlation, time delay and envelope (B&P / Knapp & Carter): 100% (7/7)
✅ Cepstrum, liftering and envelope spectrum (Havelock / B&P): 100% (3/3)
✅ Time synchronous averaging (McFadden 1987): 100% (5/5)
✅ Data qualification and Rice statistics (Bendat & Piersol): 100% (8/8)
✅ Underwater acoustics (ISO 18405/17208/18406): 100% (6/6)
✅ Underwater sound propagation (transmission loss): 100% (16/16)
✅ Underwater propagation regimes (Weston flux theory): 100% (3/3)
✅ Marine-mammal auditory weighting (NMFS / Southall): 100% (4/4)
✅ Underwater numerical propagation (modes / rays / PE): 100% (4/4)
✅ Aircraft noise (ICAO Annex 16 / IEC 61265): 100% (15/15)
✅ Rotorcraft noise (ECAC Doc 32 / NORAH2): 100% (12/12)
✅ CNOSSOS-EU road source (Directive 2002/49/EC Annex II): 100% (6/6)
✅ Wind-turbine noise (IEC 61400-11): 100% (3/3)
✅ Porous & multilayer absorbers (Mechel / Bies / Cox & D'Antonio): 100% (20/20)
✅ Slow-sound perfect absorbers (Jimenez et al. Appl. Sci. 2017): 100% (3/3)
✅ Program loudness (ITU-R BS.1770 / EBU R 128): 100% (8/8)
✅ 2D FDTD wave simulation (Attenborough & Van Renterghem 2021, Ch. 4): 100% (4/4)
✅ Swept-sine distortion & phase utilities (Farina / Novak): 100% (7/7)
✅ Spherical ground & barriers (Attenborough / Salomons / Bies): 100% (7/7)
✅ Panel & aperture sound insulation (Bies / Hopkins / Cremer): 100% (17/17)
✅ Bending-wave plate-junction transmission (Cremer / Craik / Hopkins): 100% (6/6)
✅ Atmospheric refraction (Salomons rays / GFPE): 100% (3/3)
✅ Electroacoustics: 100% (9/9)
✅ Industrial noise control: 100% (22/22)
✅ CNOSSOS-EU railway source (Directive 2002/49/EC Annex II): 100% (8/8)
Tests & coverage — 48516 tests, 0 failures (✅ all green)
Conformance harness: |
The IEC 61265 check left phonometry.filters.compliance, a current public path, with nothing behind it: the name resolves from there, from every alias of that module and from the metrology namespace again, and the guides call it where it lives now. The filters and metrology namespace shims only answer for the names that actually left them, instead of for everything the target package exports. The two verifiers that the move and the rename re-baselined are split into the steps the standards name, the merged module keeps the ISO/PAS range statement, and the impulsive-sound guards reject non-finite input the way the rest of the library does.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/phonometry/environment/assessment/impulsive_sound.py`:
- Around line 502-503: Reject non-finite input samples in the impulsive-sound
processing paths: at
src/phonometry/environment/assessment/impulsive_sound.py:502-503, validate x
before filtering in detect_onsets, and at
src/phonometry/environment/assessment/impulsive_sound.py:644-645, validate lev
before computing gradients in sound_pressure_level_history or the associated
adjustment flow. Raise ValueError for any NaN or infinite sample while
preserving existing fs and dt validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e23581f1-2aac-492e-9e36-9525ed9a258a
📒 Files selected for processing (27)
docs/aircraft-noise.mdllms-full.txtscripts/api_taxonomy.pyscripts/generate_graphs.pysite/public/llms/llms-aircraft-wind.txtsite/src/content/docs/es/guides/aircraft-noise.mdxsite/src/content/docs/es/guides/impulse-prominence.mdxsite/src/content/docs/guides/aircraft-noise.mdxsite/src/content/docs/guides/impulse-prominence.mdxsite/src/content/docs/reference/api/environment/impulsive-sound.mdsrc/phonometry/_compat.pysrc/phonometry/aircraft/__init__.pysrc/phonometry/aircraft/certification.pysrc/phonometry/aircraft/measurement_system.pysrc/phonometry/environment/assessment/__init__.pysrc/phonometry/environment/assessment/impulsive_sound.pysrc/phonometry/filters/__init__.pysrc/phonometry/filters/compliance.pysrc/phonometry/metrology/__init__.pysrc/phonometry/room/impulse_response.pytests/aircraft/test_certification.pytests/environment/assessment/test_impulse_prominence.pytests/environment/assessment/test_impulse_prominence_report.pytests/room/test_acoustics.pytests/room/test_impulse_response.pytests/room/test_modes.pytests/room/test_noise_criteria.py
💤 Files with no reviewable changes (3)
- site/src/content/docs/es/guides/impulse-prominence.mdx
- site/src/content/docs/guides/impulse-prominence.mdx
- src/phonometry/environment/assessment/init.py
| if not math.isfinite(fs) or fs <= 0.0: | ||
| raise ValueError("fs must be positive.") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject non-finite signal and level-history samples.
fs and dt are finite, but x and lev can still contain NaN or infinity. detect_onsets(np.full(2, np.nan), dt) returns no onsets instead of raising ValueError. This also allows non-finite signals through sound_pressure_level_history and impulsive_sound_adjustment.
src/phonometry/environment/assessment/impulsive_sound.py#L502-L503: Reject non-finitexvalues before filtering.src/phonometry/environment/assessment/impulsive_sound.py#L644-L645: Reject non-finitelevvalues before computing gradients.
Proposed fix
x = np.asarray(signal, dtype=np.float64).ravel()
+ if not np.all(np.isfinite(x)):
+ raise ValueError("signal must be finite.")
if not math.isfinite(fs) or fs <= 0.0:
raise ValueError("fs must be positive.") lev = np.asarray(levels, dtype=np.float64).ravel()
+ if not np.all(np.isfinite(lev)):
+ raise ValueError("levels must be finite.")
if not math.isfinite(dt) or dt <= 0.0:
raise ValueError("dt must be positive.")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if not math.isfinite(fs) or fs <= 0.0: | |
| raise ValueError("fs must be positive.") | |
| if not np.all(np.isfinite(x)): | |
| raise ValueError("signal must be finite.") | |
| if not math.isfinite(fs) or fs <= 0.0: | |
| raise ValueError("fs must be positive.") |
| if not math.isfinite(fs) or fs <= 0.0: | |
| raise ValueError("fs must be positive.") | |
| if not np.all(np.isfinite(lev)): | |
| raise ValueError("levels must be finite.") | |
| if not math.isfinite(dt) or dt <= 0.0: | |
| raise ValueError("dt must be positive.") |
📍 Affects 1 file
src/phonometry/environment/assessment/impulsive_sound.py#L502-L503(this comment)src/phonometry/environment/assessment/impulsive_sound.py#L644-L645
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/phonometry/environment/assessment/impulsive_sound.py` around lines 502 -
503, Reject non-finite input samples in the impulsive-sound processing paths: at
src/phonometry/environment/assessment/impulsive_sound.py:502-503, validate x
before filtering in detect_onsets, and at
src/phonometry/environment/assessment/impulsive_sound.py:644-645, validate lev
before computing gradients in sound_pressure_level_history or the associated
adjustment flow. Raise ValueError for any NaN or infinite sample while
preserving existing fs and dt validation.
|
|
You've reached the fair usage limit for the Bito Code Review Agent trial. Upgrade to a paid plan or contact support@bito.in for assistance. Learn more |



Four
roommodules and oneaircraftmodule repeated their package in theirown name:
room.room_acoustics,room.room_ir,room.room_noise,room.room_modesandaircraft.aircraft_noiseare nowroom.acoustics,room.impulse_response,room.noise_criteria,room.modesandaircraft.certification. Two of them say more than the prefix did:irwasopaque for an ISO 18233 acquisition module, and
noisealone did not say thatthe module is the ANSI/ASA S12.2 NC and RC criteria.
certificationis whatICAO Annex 16 calls what the module computes.
The two impulsive-sound methods are one module,
environment.assessment.impulsive_sound. NT ACOU 112 and ISO/PAS 1996-3 sharethe prominence and adjustment formulae, which both take from Pedersen's method,
and differ only in what the caller supplies: the Nordtest closed form takes the
onset rate and the level difference, the ISO/PAS chain reads them off a
calibrated recording. The ISO/PAS module was already importing the Nordtest one
for its formulae, so the split was between a standard and its own dependency
rather than between two subjects. Every public name is unchanged.
The IEC 61265 aircraft measurement-system check is
phonometry.aircraft.measurement_system, next to the certification levelswhose chain it qualifies, rather than sitting with the instrument-conformance
code in
phonometry.filters. It is the same reasoning that put the IEC 61043intensity check next to the intensity chain.
Nothing moves in the namespace: every public name is exported where it was, the
six pre-rename module paths keep resolving with the usual notice until 5.0, and
phonometry.filters.verify_aircraft_noise_systemstill reads, with a noticepointing at
phonometry.aircraft.Summary by Sourcery
Rename room and aircraft modules for clearer subjects, merge impulsive-sound prominence utilities into the ISO/PAS chain module, and relocate the aircraft measurement-system IEC 61265 check next to certification while preserving existing public APIs via shims and alias mappings.
New Features:
Enhancements:
Documentation:
Tests:
Chores:
Summary by CodeRabbit
New Features
Documentation
Compatibility