Clean up the audio_filters directory - #15087
Conversation
- Restore equal_loudness_filter.py: replace the third-party 'yulewalker' dependency with a dependency-free 'yulewalk' implementation (numpy + scipy only, both already project dependencies) so the filter works again. - Fix two bugs in iir_filter.set_coefficients: the a0-omitted branch never triggered, and the b_coeffs length error reported len(a_coeffs). - Add doctests covering valid and erroneous inputs (coefficient-length errors, yulewalk input validation). - Add Wikipedia URLs to the docstrings that were missing them. - Add two new RBJ Audio EQ Cookbook filters: make_notch and make_bandpass_peak. - Rewrite audio_filters/README.md to document every file and filter.
|
@fJpmjZdgpD, @dredonjaquana26 Your reviews, please. |
There was a problem hiding this comment.
Pull request overview
This PR cleans up and re-enables the audio_filters/ directory by restoring the equal-loudness filter without third-party dependencies, expanding filter offerings, and improving documentation and doctest coverage for filter APIs.
Changes:
- Restores
equal_loudness_filter.pywith an in-moduleyulewalk()implementation usingnumpy/scipy. - Fixes and extends
IIRFilter.set_coefficients()doctests and error handling, and adds two new RBJ-cookbook-derived filter designs. - Improves discoverability via README updates and module-level documentation links.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| audio_filters/show_response.py | Adds a module docstring with a reference URL for frequency response. |
| audio_filters/README.md | Rewrites README to enumerate modules/filters and provide a runnable example. |
| audio_filters/iir_filter.py | Fixes a0-omission handling and improves doctest coverage for coefficient validation. |
| audio_filters/equal_loudness_filter.py.broken.txt | Removes the previously-disabled equal loudness implementation. |
| audio_filters/equal_loudness_filter.py | Restores equal-loudness filter and introduces dependency-free yulewalk() plus helpers. |
| audio_filters/butterworth_filter.py | Adds Wikipedia link and introduces make_notch() and make_bandpass_peak() designs with doctests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if len(a_coeffs) < self.order + 1: | ||
| a_coeffs = [1.0, *a_coeffs] | ||
|
|
||
| if len(a_coeffs) != self.order + 1: |
| if np.any(np.diff(frequencies) < 0): | ||
| msg = "frequencies must be in increasing order" | ||
| raise ValueError(msg) |
| # pad the data to nyquist | ||
| curve_freqs = np.array(data["frequencies"] + [max(20000.0, samplerate / 2)]) | ||
| curve_gains = np.array(data["gains"] + [140]) |
| return filt | ||
|
|
||
|
|
||
| def make_notch( |
There was a problem hiding this comment.
Can we move make_notch() and make_bandpass_peak() to their own files? Or is there some reason that they should remain in this file?
make_notch() contains many short, cryptic variable names that make the algorithm read like a chemistry formula. Experts might be comfortable with that, but it might be difficult for new developers to understand. Can any of these be renamed to more self-documenting variable names that would help visitors follow the complexity?
There was a problem hiding this comment.
Moving them to a new file is not necessary. I now see that this file contains lots of filters, so we can continue that approach.
…set_coefficients guard - Add a shared notation guide to butterworth_filter so the single-letter RBJ-cookbook names (w0/alpha/a0..a2/b0..b2) are self-explanatory across every filter instead of diverging in one function. - Comment make_notch's feed-forward/feed-back sections in plain English. - set_coefficients only fills the optional a_0 when exactly one coefficient is missing, so genuinely too-short inputs raise with their real length (new regression doctest).
|
Thanks @cclauss — pushed in 764c115. On the So instead of diverging locally, I made the shared notation self-documenting:
Happy to go further and rename across the whole file if you'd prefer full self-documenting names everywhere — I just didn't want to touch the other filters unasked in this PR. Also addressed the Copilot review while I was in there: |
Describe your change:
This cleans up the
audio_filters/directory as requested in #15081. Working through the checklist from that thread:equal_loudness_filter.py. The file was disabled (.broken.txt) because it imported the third-partyyulewalkerpackage, which isn't a project dependency. I replaced that import with a dependency-freeyulewalk()implementation of the modified Yule-Walker method that uses onlynumpyandscipy(both already inpyproject.toml). The restoredEqualLoudnessFilternow constructs and processes samples again, and the designed filter is verified stable in a doctest.IIRFilter.set_coefficients: the "leave outa_0" branch (len(a_coeffs) < self.order) never triggered, and theb_coeffslength-error message reportedlen(a_coeffs). Added doctests covering both valid and erroneous inputs (coefficient-lengthValueErrors,yulewalkinput validation for mismatched lengths and non-increasing frequencies), plus doctests for the new helpers.butterworth_filter.py,equal_loudness_filter.py,show_response.py).README.md. Rewrote it to describe every file and every filter in the directory, with a runnable "Try it out" example and links encouraging readers to learn more.make_notch(band-reject, e.g. for mains hum) andmake_bandpass_peak(the constant-0 dB-peak-gain band-pass variant from the RBJ Audio EQ Cookbook), both with type hints and doctests.All modules pass
python -m pytest --doctest-modules audio_filters/,ruff check, andruff format --check. Credit for the original equal-loudness filter design remains with David Robinson (2001), as noted in the docstring.Closes part of the maintenance exercise in #15081.
Checklist: