Geometry drawings: secondary set-ups across building, materials and rooms - #386
Merged
Conversation
…ooms Nine more to-scale drawings, each with the realistic dimensions of its standard rig or textbook case: - Composite facade elevation with the element areas to scale (FacadePredictionResult retains its elements). - Mass-spring-mass double wall: 12,5 mm plasterboard leaves on a 100 mm cavity, resonance annotated (double-wall results retain mass1/mass2/gap). - Plate junctions L/T1/T2/X following the junction_transmission conventions (plate 1 continuous for T1/X, the perpendicular pair continuous for T2/X); results retain both thicknesses. - In-situ road absorption set-up: source at 1,25 m, microphone at 0,25 m, the 1,34 m sampled radius of the 5 ms window. - Dynamic-stiffness resonance rig: 200 mm square specimen under the 8 kg load plate. - Free-field diffusion goniometer in plan: 37 microphones on the 5 m arc, source at 10 m. - Baffled radiating plate (RadiationEfficiencyResult already carries its geometry). - Open-plan measurement line with rD/rP marked (OpenPlanResult retains the microphone positions). - Face-to-face p-p intensity probe with the 12 mm spacer (IntensityResult retains the spacing). Nine public plot_* functions, retention fields on six results (trailing, defaulted), 36 generated SVG variants embedded in nine guides (EN/ES and the GitHub mirrors, including the first figures of the junction-transmission and vibration-sound-power guides), curated table and generated API pages updated, and 10 new tests.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @jmrplens, your pull request is larger than the review limit of 150000 diff characters
github-actions
Bot
added
documentation
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds scale geometry plotting helpers for acoustic measurement and construction setups, stores geometry metadata in result objects for redraws, exposes the helpers publicly, generates SVG figures, updates English and Spanish documentation, and adds validation tests. ChangesGeometry plotting API
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #386 +/- ##
==========================================
- Coverage 96.83% 96.82% -0.01%
==========================================
Files 219 219
Lines 31103 31391 +288
==========================================
+ Hits 30118 30395 +277
- Misses 985 996 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 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 `@docs/api-reference.md`:
- Line 824: Update the plot_goniometer_geometry API reference entry to include
the sample_width parameter with its default value of 0.6 m alongside the other
documented drawing parameters, preserving the existing public signature and
example.
- Line 234: Standardize the user-facing terminology in the documented
facade-related entries, including the row for facade_sound_reduction and the
entries around the additional referenced section, to consistently use “façade”
instead of mixing it with “facade”. Preserve code symbols, parameter names, and
examples unchanged.
In `@docs/insulation-prediction.md`:
- Around line 400-404: Update the surrounding prose in the composite explanation
to use “façade” consistently, while preserving the code identifier
`plot_facade_elements` unchanged.
In `@docs/junction-transmission.md`:
- Around line 19-41: Align the runnable examples with the displayed T1 geometry
by changing the junction_transmission call in docs/junction-transmission.md
(lines 19-41) and site/src/content/docs/guides/junction-transmission.mdx (lines
44-65) from T2 to T1; leave the SVG and alt text unchanged.
In `@site/src/content/docs/es/guides/junction-transmission.mdx`:
- Around line 45-69: Keep the junction type consistent between the ThemeImage
alt text and the junction_transmission example: update the T1 reference in the
accessible description or change the example’s "T2" argument so both describe
the same setup.
In `@site/src/content/docs/guides/insulation-prediction.mdx`:
- Around line 477-481: Update the natural-language occurrence of “facade” in the
description around plot_facade_elements and fac.plot_geometry() to “façade”;
preserve the unaccented spelling only within the Python identifiers.
In `@site/src/content/docs/reference/api/building/facade-prediction.md`:
- Line 190: Complete the retained-geometry attribute tables: in
site/src/content/docs/reference/api/building/facade-prediction.md:190-190,
document elements as tuple[FacadeElement, ...] and state its None behavior; in
site/src/content/docs/reference/api/building/panel-transmission.md:304-306,
document mass1, mass2, and gap with their units and None behavior, clarifying
the plot_geometry() retention contract.
In `@site/src/content/docs/reference/api/materials/road-absorption.md`:
- Around line 413-418: Update the Attributes tables for the public result types:
in site/src/content/docs/reference/api/materials/road-absorption.md:413-418,
document source_height and mic_height with units and None/retained-geometry
behavior; in site/src/content/docs/reference/api/power/intensity.md:196,
document spacing with units and its role in probe redraws and bandwidth
calculations; in site/src/content/docs/reference/api/rooms/open-plan.md:77-83,
document positions_m as one-dimensional metre-valued data and describe its None
behavior; and in
site/src/content/docs/reference/api/vibration/junction-transmission.md:366-367,
document thickness1 and thickness2 in metres and explain their effect on
geometry redraws.
In `@site/src/content/docs/reference/api/materials/scattering-diffusion.md`:
- Around line 585-588: Update the API description’s compound modifier from “5
degree resolution” to “5-degree resolution,” leaving the surrounding description
unchanged.
In `@src/phonometry/_plot/geometry.py`:
- Around line 2306-2311: Update the rectangle-rendering loops in
plot_facade_elements and plot_double_wall_geometry so every _material_rect call
receives the provided kwargs, rather than restricting them to the first tile or
leaf. Preserve each call’s existing geometry and fill selection while forwarding
the same options to all element and leaf rectangles.
- Around line 2292-2297: Update the area collection in the tile geometry path to
default to 0.1 only when an element’s area attribute is missing or None, while
preserving explicit area=0.0 for the existing positivity validation. Keep the
current ValueError behavior for zero or negative areas and the existing fallback
for elements without an area.
- Around line 2700-2767: Validate sample_width in plot_goniometer_geometry
alongside source_distance and receiver_radius, requiring it to be strictly
positive and raising a clear ValueError otherwise. Preserve the existing
validation and rendering behavior for valid inputs.
🪄 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: 3a104d0c-533c-418e-8c7b-d931647fb226
⛔ Files ignored due to path filters (36)
.github/images/diffusion_goniometer_geometry.svgis excluded by!**/*.svg.github/images/diffusion_goniometer_geometry_dark.svgis excluded by!**/*.svg.github/images/diffusion_goniometer_geometry_es.svgis excluded by!**/*.svg.github/images/diffusion_goniometer_geometry_es_dark.svgis excluded by!**/*.svg.github/images/double_wall_geometry.svgis excluded by!**/*.svg.github/images/double_wall_geometry_dark.svgis excluded by!**/*.svg.github/images/double_wall_geometry_es.svgis excluded by!**/*.svg.github/images/double_wall_geometry_es_dark.svgis excluded by!**/*.svg.github/images/dynamic_stiffness_rig_geometry.svgis excluded by!**/*.svg.github/images/dynamic_stiffness_rig_geometry_dark.svgis excluded by!**/*.svg.github/images/dynamic_stiffness_rig_geometry_es.svgis excluded by!**/*.svg.github/images/dynamic_stiffness_rig_geometry_es_dark.svgis excluded by!**/*.svg.github/images/facade_elevation_geometry.svgis excluded by!**/*.svg.github/images/facade_elevation_geometry_dark.svgis excluded by!**/*.svg.github/images/facade_elevation_geometry_es.svgis excluded by!**/*.svg.github/images/facade_elevation_geometry_es_dark.svgis excluded by!**/*.svg.github/images/insitu_setup_geometry.svgis excluded by!**/*.svg.github/images/insitu_setup_geometry_dark.svgis excluded by!**/*.svg.github/images/insitu_setup_geometry_es.svgis excluded by!**/*.svg.github/images/insitu_setup_geometry_es_dark.svgis excluded by!**/*.svg.github/images/junction_plate_geometry.svgis excluded by!**/*.svg.github/images/junction_plate_geometry_dark.svgis excluded by!**/*.svg.github/images/junction_plate_geometry_es.svgis excluded by!**/*.svg.github/images/junction_plate_geometry_es_dark.svgis excluded by!**/*.svg.github/images/open_plan_line_geometry.svgis excluded by!**/*.svg.github/images/open_plan_line_geometry_dark.svgis excluded by!**/*.svg.github/images/open_plan_line_geometry_es.svgis excluded by!**/*.svg.github/images/open_plan_line_geometry_es_dark.svgis excluded by!**/*.svg.github/images/pp_probe_geometry.svgis excluded by!**/*.svg.github/images/pp_probe_geometry_dark.svgis excluded by!**/*.svg.github/images/pp_probe_geometry_es.svgis excluded by!**/*.svg.github/images/pp_probe_geometry_es_dark.svgis excluded by!**/*.svg.github/images/radiation_plate_geometry.svgis excluded by!**/*.svg.github/images/radiation_plate_geometry_dark.svgis excluded by!**/*.svg.github/images/radiation_plate_geometry_es.svgis excluded by!**/*.svg.github/images/radiation_plate_geometry_es_dark.svgis excluded by!**/*.svg
📒 Files selected for processing (51)
docs/api-reference.mddocs/dynamic-stiffness.mddocs/insulation-prediction.mddocs/intensity.mddocs/junction-transmission.mddocs/panel-sound-insulation.mddocs/room-acoustics.mddocs/surface-scattering.mddocs/vibration-sound-power.mdscripts/api_taxonomy.pyscripts/generate_graphs.pysite/src/content/docs/es/guides/dynamic-stiffness.mdxsite/src/content/docs/es/guides/insulation-prediction.mdxsite/src/content/docs/es/guides/intensity.mdxsite/src/content/docs/es/guides/junction-transmission.mdxsite/src/content/docs/es/guides/panel-sound-insulation.mdxsite/src/content/docs/es/guides/room-acoustics.mdxsite/src/content/docs/es/guides/surface-scattering.mdxsite/src/content/docs/es/guides/vibration-sound-power.mdxsite/src/content/docs/guides/dynamic-stiffness.mdxsite/src/content/docs/guides/insulation-prediction.mdxsite/src/content/docs/guides/intensity.mdxsite/src/content/docs/guides/junction-transmission.mdxsite/src/content/docs/guides/panel-sound-insulation.mdxsite/src/content/docs/guides/room-acoustics.mdxsite/src/content/docs/guides/surface-scattering.mdxsite/src/content/docs/guides/vibration-sound-power.mdxsite/src/content/docs/reference/api/building/facade-prediction.mdsite/src/content/docs/reference/api/building/panel-transmission.mdsite/src/content/docs/reference/api/materials/dynamic-stiffness.mdsite/src/content/docs/reference/api/materials/road-absorption.mdsite/src/content/docs/reference/api/materials/scattering-diffusion.mdsite/src/content/docs/reference/api/power/intensity.mdsite/src/content/docs/reference/api/rooms/open-plan.mdsite/src/content/docs/reference/api/vibration/junction-transmission.mdsite/src/content/docs/reference/api/vibration/radiation-efficiency.mdsrc/phonometry/__init__.pysrc/phonometry/_plot/geometry.pysrc/phonometry/building/__init__.pysrc/phonometry/building/facade_prediction.pysrc/phonometry/building/panel_transmission.pysrc/phonometry/emission/__init__.pysrc/phonometry/emission/intensity.pysrc/phonometry/materials/__init__.pysrc/phonometry/materials/road_absorption.pysrc/phonometry/room/__init__.pysrc/phonometry/room/open_plan.pysrc/phonometry/vibration/__init__.pysrc/phonometry/vibration/junction_transmission.pysrc/phonometry/vibration/radiation_efficiency.pytests/test_secondary_geometry_plots.py
Contributor
Numerical conformance report✅ 427/427 conformance checks pass across 53 domains and 278 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% (6/6)
✅ Room acoustics: 100% (12/12)
✅ Psychoacoustics: 100% (12/12)
✅ Speech transmission (IEC 60268-16): 100% (10/10)
✅ System measurement (Golay / Kirkeby / Mueller-Massarani): 100% (5/5)
✅ Intensity & sound power: 100% (6/6)
✅ Room & building acoustics: 100% (52/52)
✅ 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% (7/7)
✅ 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% (15/15)
✅ 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)
✅ Wind-turbine noise (IEC 61400-11): 100% (3/3)
✅ Porous & multilayer absorbers (Mechel / Bies / Cox & D'Antonio): 100% (10/10)
✅ 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% (2/2)
✅ 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% (11/11)
✅ Bending-wave plate-junction transmission (Cremer / Craik / Hopkins): 100% (6/6)
✅ Atmospheric refraction (Salomons rays / GFPE): 100% (3/3)
✅ Electroacoustics: 100% (6/6)
✅ Industrial noise control: 100% (9/9)
Tests & coverage — 33852 tests, 0 failures (✅ all green)
Conformance harness: |
- The T-junction figure alt texts now name T2, matching the drawn geometry and the runnable example. - Retained-geometry fields documented on all six result docstrings (and the generated API pages regenerated). - plot_facade_elements rejects an explicit zero area instead of silently substituting the nominal tile, and its kwargs contract now names the first element rectangle; plot_goniometer_geometry validates sample_width and documents it in the curated table. - Facade prose spelling unified with the surrounding sections.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/phonometry/_plot/geometry.py (2)
2744-2746: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid duplicate
zorderarguments.
zorderis a validax.scatterkeyword, so passing bothzorder=5and**kwargsraisesTypeErrorwhen callers supply it. Usekwargs.setdefault("zorder", 5)"and omit the explicit keyword.🤖 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/_plot/geometry.py` around lines 2744 - 2746, Update the scatter call in the surrounding plotting function to set the default z-order through kwargs.setdefault("zorder", 5), then remove the explicit zorder argument from ax.scatter so caller-provided values are preserved without duplicate keyword errors.
2740-2743: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEnsure arbitrary angular steps cover the full receiver semicircle.
np.arange(-90.0, 90.0 + 0.5 * angular_step, angular_step)stops before 90° whenever the 180° span is not evenly divisible byangular_step(for example,angular_step=7ends at 92°). Since these points are passed directly toax.scatter(xs, ys)andax.plot(xs, ys), the dashed arc is drawn using points that do not reach the intended semicircle endpoint; append the 90°, -90° endpoint or require180 % angular_step == 0.🤖 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/_plot/geometry.py` around lines 2740 - 2743, Update the angle generation around angles, xs, and ys so every arbitrary angular_step includes the exact -90° and 90° receiver semicircle endpoints, preventing overshoot or gaps from np.arange; preserve the existing intermediate sampling used by ax.scatter and ax.plot.
🤖 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/_plot/geometry.py`:
- Around line 2292-2300: Update the plotting geometry validation around the area
collection and the related source_distance, receiver_radius, and sample_width
inputs to convert each value once, reject non-finite values with np.isfinite
alongside the existing positivity checks, and reuse the converted numeric values
for subsequent calculations. Preserve the nominal 0.1 fallback for elements
without an area.
---
Outside diff comments:
In `@src/phonometry/_plot/geometry.py`:
- Around line 2744-2746: Update the scatter call in the surrounding plotting
function to set the default z-order through kwargs.setdefault("zorder", 5), then
remove the explicit zorder argument from ax.scatter so caller-provided values
are preserved without duplicate keyword errors.
- Around line 2740-2743: Update the angle generation around angles, xs, and ys
so every arbitrary angular_step includes the exact -90° and 90° receiver
semicircle endpoints, preventing overshoot or gaps from np.arange; preserve the
existing intermediate sampling used by ax.scatter and ax.plot.
🪄 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: f45dab03-73a6-4ec3-9916-7a51475e30ad
📒 Files selected for processing (20)
docs/api-reference.mddocs/insulation-prediction.mddocs/junction-transmission.mdsite/src/content/docs/es/guides/junction-transmission.mdxsite/src/content/docs/guides/insulation-prediction.mdxsite/src/content/docs/guides/junction-transmission.mdxsite/src/content/docs/reference/api/building/facade-prediction.mdsite/src/content/docs/reference/api/building/panel-transmission.mdsite/src/content/docs/reference/api/materials/road-absorption.mdsite/src/content/docs/reference/api/materials/scattering-diffusion.mdsite/src/content/docs/reference/api/power/intensity.mdsite/src/content/docs/reference/api/rooms/open-plan.mdsite/src/content/docs/reference/api/vibration/junction-transmission.mdsrc/phonometry/_plot/geometry.pysrc/phonometry/building/facade_prediction.pysrc/phonometry/building/panel_transmission.pysrc/phonometry/emission/intensity.pysrc/phonometry/materials/road_absorption.pysrc/phonometry/room/open_plan.pysrc/phonometry/vibration/junction_transmission.py
Comment on lines
+2292
to
+2300
| areas: list[float] = [] | ||
| for element in tiles: | ||
| area = getattr(element, "area", None) | ||
| if area is None: | ||
| areas.append(0.1) # nominal tile for dn_e-rated elements | ||
| elif float(area) <= 0.0: | ||
| raise ValueError("Element areas must be positive.") | ||
| else: | ||
| areas.append(float(area)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'geometry.py' . || true
echo "== relevant sections =="
if [ -f src/phonometry/_plot/geometry.py ]; then
wc -l src/phonometry/_plot/geometry.py
sed -n '2260,2315p' src/phonometry/_plot/geometry.py | cat -n -v
echo "---"
sed -n '2700,2755p' src/phonometry/_plot/geometry.py | cat -n -v
echo "--- outline relevant symbols ---"
ast-grep outline src/phonometry/_plot/geometry.py --match 'render_goniometer_grid' --view expanded || true
ast-grep outline src/phonometry/_plot/geometry.py --match 'render_element_grid' --view expanded || true
fi
echo "== search references to finite/Area/source/receiver/sample =="
rg -n "source_distance|receiver_radius|sample_width|areas =|Element areas|nullish|isfinite|np\.isfinite" src/phonometry/_plot/geometry.pyRepository: jmrplens/phonometry
Length of output: 7273
🌐 Web query:
Matplotlib artists NaN coordinates invalid artists behavior error message
💡 Result:
In Matplotlib, the handling of NaN (Not-a-Number) values in artist coordinates is inconsistent and often backend- or context-dependent, sometimes resulting in error messages like "ValueError: cannot convert float NaN to integer" [1][2][3]. While Matplotlib is generally designed to handle NaN values gracefully in many plot types (e.g., Line2D plots where NaN breaks the line, or scatter plots where they are ignored) [4][5], passing NaN coordinates to certain artists or layout engines—such as those involved in text annotation or tight layout calculations—can trigger failures [1][2]. Common causes and behaviors include: 1. Layout Engines and BBox Calculations: Errors frequently arise during layout operations like plt.tight_layout() or when saving with bbox_inches='tight', as these routines may attempt to calculate bounding boxes or tick spacing for artists with undefined (NaN) positions [1][2]. This often causes a "ValueError: cannot convert float NaN to integer" because the underlying code attempts to convert these non-finite coordinates into integer pixel positions [1][2][3]. 2. Artist Inconsistency: While some artists correctly skip invalid coordinates, others may drop them entirely or, in older or specific configurations, attempt to process them, leading to unexpected behavior [4][3]. 3. Recommended Workarounds: - If an artist is causing layout errors due to NaN coordinates, you can prevent it from being included in these calculations by setting artist.set_in_layout(False) [6][1]. - Ensure that corresponding dependent data (like 'z' values in contours or color arrays in scatters) is masked or NaN-aligned when input coordinates contain NaNs to maintain consistency [7]. - Use proxy artists for legends or other features if you need to represent objects without explicitly plotting them with NaN-valued coordinates [6]. If you encounter this error, checking whether the failing artist is involved in layout calculations (like annotate or scatter with clip_on=False) is the most effective troubleshooting step [1][3].
Citations:
- 1: Regression when calling annotate with nan values for the position matplotlib/matplotlib#12648
- 2: calling annotate with nan values for the position still gives error after 3.0.2 matplotlib/matplotlib#13276
- 3: creating PathCollection proxy artist with %matplotlib inline raises ValueError: cannot convert float NaN to integer matplotlib/matplotlib#10360
- 4: scatter() should not drop data points at nonfinite coordinates matplotlib/matplotlib#12801
- 5: https://matplotlib.org/stable/gallery/lines_bars_and_markers/masked_demo.html
- 6: https://discourse.matplotlib.org/t/is-it-ok-to-use-nan-for-patch-rect-coordinates/25993
- 7: Contour behaviour changed with NaN coordinates matplotlib/matplotlib#20443
Reject non-finite dimensions before plotting.
nan and inf pass <= 0.0, so facade areas can produce NaN rectangle dimensions and goniometer coordinates/dimensions can become invalid artists. Add np.isfinite(...) checks for areas, source_distance, receiver_radius, and sample_width; use the converted numeric values afterwards to avoid a second parsing step.
🤖 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/_plot/geometry.py` around lines 2292 - 2300, Update the
plotting geometry validation around the area collection and the related
source_distance, receiver_radius, and sample_width inputs to convert each value
once, reject non-finite values with np.isfinite alongside the existing
positivity checks, and reuse the converted numeric values for subsequent
calculations. Preserve the nominal 0.1 fallback for elements without an area.
|
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.
Summary
Third and final batch of the to-scale geometry API: the secondary set-ups. Every drawing uses the realistic dimensions of its standard rig or classic textbook case rather than invented numbers, and every result that can retain its defining geometry now does.
What is drawn
plot_facade_elements,FacadePredictionResult.plot_geometry()plot_double_wall_geometry,SoundReductionResult.plot_geometry()plot_junction_geometry,JunctionTransmissionResult.plot_geometry()plot_insitu_geometry,InsituAbsorptionResult.plot_geometry()plot_dynamic_stiffness_rigplot_goniometer_geometryplot_plate_geometry,RadiationEfficiencyResult.plot_geometry()plot_open_plan_geometry,OpenPlanResult.plot_geometry()plot_pp_probe_geometry,IntensityResult.plot_geometry()The junction drawings follow the module's own convention exactly: plate 1 is the continuous pair for T1/X and the perpendicular plates are the continuous pair for T2/X.
Verification
Summary by CodeRabbit
New Features
Documentation
Tests