docs: clarify octave band frequency resolution#48
Conversation
Reviewer's GuideDocumentation-only README updates that clarify octavefilter as a time‑domain fractional‑octave filter bank, explain standardized band spacing versus FFT bin spacing with a Welch-based example, and update the quick API reference for getansifrequencies to include nominal band labels, along with corresponding TOC and section title cleanups. Sequence diagram for Welch example using PyOctaveBand band edgessequenceDiagram
actor User
participant Script
participant pyoctaveband
participant scipy_signal as "scipy.signal"
participant numpy as numpy
User ->> Script: Configure fs, x
Script ->> pyoctaveband: octavefilter(x, fs, fraction=3, limits=[12, 20000])
pyoctaveband -->> Script: levels, centers
Script ->> pyoctaveband: getansifrequencies(fraction=3, limits=[12, 20000])
pyoctaveband -->> Script: fc, fl, fu, labels
Script ->> scipy_signal: welch(x, fs, window, nperseg, noverlap, scaling)
scipy_signal -->> Script: freq_bins, psd
Script ->> numpy: argmin(abs(fc - 1000.0))
numpy -->> Script: band_index
Script ->> Script: in_band = (freq_bins >= fl[band_index]) and (freq_bins <= fu[band_index])
Script ->> User: Print selected label, bin spacing, and in_band Welch bins
Class diagram for updated getansifrequencies APIclassDiagram
class pyoctaveband {
+octavefilter(x, fs, fraction, limits, sigbands) : levels, centers
+getansifrequencies(fraction, limits) : fc, fl, fu, labels
+normalizedfreq(fraction) : freqs
}
class OctaveBands {
+fraction : int
+limits : float[]
+centers : float[]
+lower_edges : float[]
+upper_edges : float[]
+labels : string[]
}
pyoctaveband ..> OctaveBands : returns data compatible with
Flow diagram for choosing octavefilter vs Welch FFT analysisflowchart TD
Start([Start analysis])
Q1{Need standardized<br>fractional_octave<br>levels?}
Start --> Q1
Q1 -- Yes --> UseOctave[Use octavefilter<br>with fraction and limits]
Q1 -- No --> Q2{Need narrowband<br>tonal inspection?}
UseOctave --> Bands[Interpret results as<br>one level per<br>fractional_octave band]
Q2 -- Yes --> UseWelch[Run Welch or FFT<br>on original signal]
Q2 -- No --> OtherMethods[Consider other<br>analysis methods]
UseWelch --> Edges[Call getansifrequencies<br>to obtain band centers and<br>lower_upper edges]
Edges --> Mask[Use band edges as masks<br>to select FFT_Welch bins<br>inside each band]
Bands --> End([End])
Mask --> End
OtherMethods --> End
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="README.md" line_range="29" />
<code_context>
+ - [Gallery of Filter Bank Responses](#gallery-of-filter-bank-responses)
+ - [🔊 Acoustic Weighting (A, C, Z)](#-acoustic-weighting-a-c-z)
+ - [⏱️ Time Weighting and Integration](#️-time-weighting-and-integration)
+ - [⚡ Performance: Multichannel \& Vectorization](#-performance-multichannel-vectorization)
+ - [🔍 Filter Usage and Examples](#-filter-usage-and-examples)
+ - [1. Butterworth (`butter`)](#1-butterworth-butter)
</code_context>
<issue_to_address>
**issue (bug_risk):** The updated anchor for the Performance section likely no longer matches the generated heading ID, which can break the TOC link.
Previously this link used `#-performance-multichannel--vectorization`, but now has only one hyphen between `multichannel` and `vectorization`. For a heading like `⚡ Performance: Multichannel & Vectorization`, the generated ID is usually `-performance-multichannel--vectorization`. Please either restore the previous anchor or verify the actual generated ID so the TOC link continues to work.
</issue_to_address>
### Comment 2
<location path="README.md" line_range="46" />
<code_context>
+ - [Octave Band Frequencies (ANSI S1.11 / IEC 61260)](#octave-band-frequencies-ansi-s111-iec-61260)
+ - [Frequency Resolution vs FFT Bin Spacing](#frequency-resolution-vs-fft-bin-spacing)
+ - [Magnitude Responses |H(jw)|](#magnitude-responses-hjw)
+ - [Filter Bank Design \& Numerical Stability](#filter-bank-design-numerical-stability)
+ - [Weighting Curves (IEC 61672-1)](#weighting-curves-iec-61672-1)
- [Time Integration](#time-integration)
</code_context>
<issue_to_address>
**issue (bug_risk):** The anchor for the "Filter Bank Design & Numerical Stability" section may not match the heading’s generated ID.
For this heading, GitHub’s generated ID is likely `filter-bank-design--numerical-stability` (double hyphen for the `&`), but the TOC uses `#filter-bank-design-numerical-stability`. Please confirm the actual generated ID and update the link so it matches.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| - [Gallery of Filter Bank Responses](#gallery-of-filter-bank-responses) | ||
| - [🔊 Acoustic Weighting (A, C, Z)](#-acoustic-weighting-a-c-z) | ||
| - [⏱️ Time Weighting and Integration](#️-time-weighting-and-integration) | ||
| - [⚡ Performance: Multichannel \& Vectorization](#-performance-multichannel-vectorization) |
There was a problem hiding this comment.
issue (bug_risk): The updated anchor for the Performance section likely no longer matches the generated heading ID, which can break the TOC link.
Previously this link used #-performance-multichannel--vectorization, but now has only one hyphen between multichannel and vectorization. For a heading like ⚡ Performance: Multichannel & Vectorization, the generated ID is usually -performance-multichannel--vectorization. Please either restore the previous anchor or verify the actual generated ID so the TOC link continues to work.
| - [Octave Band Frequencies (ANSI S1.11 / IEC 61260)](#octave-band-frequencies-ansi-s111-iec-61260) | ||
| - [Frequency Resolution vs FFT Bin Spacing](#frequency-resolution-vs-fft-bin-spacing) | ||
| - [Magnitude Responses |H(jw)|](#magnitude-responses-hjw) | ||
| - [Filter Bank Design \& Numerical Stability](#filter-bank-design-numerical-stability) |
There was a problem hiding this comment.
issue (bug_risk): The anchor for the "Filter Bank Design & Numerical Stability" section may not match the heading’s generated ID.
For this heading, GitHub’s generated ID is likely filter-bank-design--numerical-stability (double hyphen for the &), but the TOC uses #filter-bank-design-numerical-stability. Please confirm the actual generated ID and update the link so it matches.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request significantly enhances the documentation in README.md by reorganizing the Table of Contents, updating the getansifrequencies API reference, and adding a comprehensive explanation of frequency resolution in filter banks compared to FFT methods. Review feedback identified several technical improvements for the documentation, including fixing broken Markdown anchor links caused by ampersand slugging, defining missing variables in code snippets to ensure they are runnable, and using more robust methods to determine signal length for multichannel support in examples.
| - [Gallery of Filter Bank Responses](#gallery-of-filter-bank-responses) | ||
| - [🔊 Acoustic Weighting (A, C, Z)](#-acoustic-weighting-a-c-z) | ||
| - [⏱️ Time Weighting and Integration](#️-time-weighting-and-integration) | ||
| - [⚡ Performance: Multichannel \& Vectorization](#-performance-multichannel-vectorization) |
There was a problem hiding this comment.
The anchor link for this header is likely broken. In GitHub's Markdown renderer, the ampersand (&) is removed, and the spaces surrounding it are converted to dashes, resulting in a double dash (--). Also, escaping the ampersand with a backslash (\&) is unnecessary and may cause the backslash to be rendered literally in some environments.
| - [⚡ Performance: Multichannel \& Vectorization](#-performance-multichannel-vectorization) | |
| - [⚡ Performance: Multichannel & Vectorization](#-performance-multichannel--vectorization) |
| - [Octave Band Frequencies (ANSI S1.11 / IEC 61260)](#octave-band-frequencies-ansi-s111-iec-61260) | ||
| - [Frequency Resolution vs FFT Bin Spacing](#frequency-resolution-vs-fft-bin-spacing) | ||
| - [Magnitude Responses |H(jw)|](#magnitude-responses-hjw) | ||
| - [Filter Bank Design \& Numerical Stability](#filter-bank-design-numerical-stability) | ||
| - [Weighting Curves (IEC 61672-1)](#weighting-curves-iec-61672-1) | ||
| - [Time Integration](#time-integration) |
There was a problem hiding this comment.
These sub-items in the Theoretical Background section are missing their anchor links. Additionally, the link for 'Filter Bank Design & Numerical Stability' should account for the double dash generated by the ampersand slugging.
| - [Octave Band Frequencies (ANSI S1.11 / IEC 61260)](#octave-band-frequencies-ansi-s111-iec-61260) | |
| - [Frequency Resolution vs FFT Bin Spacing](#frequency-resolution-vs-fft-bin-spacing) | |
| - [Magnitude Responses |H(jw)|](#magnitude-responses-hjw) | |
| - [Filter Bank Design \& Numerical Stability](#filter-bank-design-numerical-stability) | |
| - [Weighting Curves (IEC 61672-1)](#weighting-curves-iec-61672-1) | |
| - [Time Integration](#time-integration) | |
| - [Octave Band Frequencies (ANSI S1.11 / IEC 61260)](#octave-band-frequencies-ansi-s111-iec-61260) | |
| - [Frequency Resolution vs FFT Bin Spacing](#frequency-resolution-vs-fft-bin-spacing) | |
| - [Magnitude Responses |H(jw)|](#magnitude-responses-hjw) | |
| - [Filter Bank Design & Numerical Stability](#filter-bank-design--numerical-stability) | |
| - [Weighting Curves (IEC 61672-1)](#weighting-curves-iec-61672-1) | |
| - [Time Integration](#time-integration) |
| from pyoctaveband import octavefilter, getansifrequencies | ||
|
|
||
| fs = 100_000 | ||
| x = pressure_signal_pa # 1D pressure signal in Pa |
There was a problem hiding this comment.
The variable pressure_signal_pa is not defined in this snippet, which prevents the example from being runnable as-is. Providing a synthetic signal generation step makes the documentation more helpful for users.
| x = pressure_signal_pa # 1D pressure signal in Pa | |
| x = np.random.randn(fs) # Example 1D pressure signal in Pa |
| fc, fl, fu, labels = getansifrequencies(fraction=3, limits=[12, 20_000]) | ||
|
|
||
| # Narrowband Welch estimate on the original signal. | ||
| nperseg = min(2**15, len(x)) |
There was a problem hiding this comment.
Using len(x) on a NumPy array returns the size of the first dimension. If the input signal is multichannel (2D), this would return the number of channels instead of the number of samples, leading to an incorrect nperseg for the Welch estimate. Using x.shape[-1] or x.size (for 1D) is more robust.
| nperseg = min(2**15, len(x)) | |
| nperseg = min(2**15, x.shape[-1]) |
|
CI Results 🚀Test Summary
Technical Benchmark Summary📊 View Benchmark DetailsPyOctaveBand: Technical Benchmark ReportGenerated: 2026-05-09 11:37:08 1. Test Signal Parameters
2. Crossover (Linkwitz-Riley)
3. Precision & Isolation
4. Performance
|
1 participant
Summary
octavefilteris a time-domain fractional-octave filter bank, not an FFT/Welch spectrum estimator.getansifrequencies()to include nominal labels.Verification
git diff --check -- README.md.Summary by Sourcery
Clarify the conceptual difference between fractional-octave filtering and FFT/Welch spectral estimation, and extend the README with guidance on standardized band definitions and their use with PyOctaveBand.
Documentation:
octavefilteris a time-domain fractional-octave filter bank without FFT-style frequency resolution and explain fractional-octave band spacing.getansifrequenciesband edges for tonal inspection within standardized bands.getansifrequencies()to include nominal band labels in its return values.