Add configurable time weighting initial state#49
Merged
Conversation
Reviewer's GuideAdds configurable initial-state handling for time_weighting (including multichannel and impulse modes), centralizes and exposes the package version, improves filter/utility coverage, and switches the release workflow to tag-based publishing with version validation. Class diagram for updated time_weighting API and version handlingclassDiagram
class parametric_filters {
+time_weighting(x, fs, mode, initial_state) np.ndarray
+weighting_filter(x, fs, curve) np.ndarray
+linkwitz_riley(x, fs, freq, order) tuple
}
class time_weighting_function {
+time_weighting(x, fs, mode, initial_state) np.ndarray
}
class prepare_time_weighting_initial_state_function {
+_prepare_time_weighting_initial_state(x_sq, initial_state) np.ndarray
}
class apply_impulse_kernel_function {
+_apply_impulse_kernel(x_t, alpha_rise, alpha_fall, initial_state) np.ndarray
}
class _version_module {
+__version__ : str
}
class package_init_module {
+__version__ : str
+__all__ : list
}
parametric_filters ..> time_weighting_function : defines
parametric_filters ..> prepare_time_weighting_initial_state_function : defines
parametric_filters ..> apply_impulse_kernel_function : defines
time_weighting_function --> prepare_time_weighting_initial_state_function : uses
time_weighting_function --> apply_impulse_kernel_function : uses for_impulse_mode
package_init_module --> _version_module : imports __version__
class fast_slow_time_weighting_behavior {
+alpha : float
+zi : np.ndarray
+y : np.ndarray
}
class impulse_time_weighting_behavior {
+alpha_rise : float
+alpha_fall : float
+x_t : np.ndarray
+y_t : np.ndarray
+initial_kernel : np.ndarray
}
time_weighting_function --> fast_slow_time_weighting_behavior : configures for_fast_slow
time_weighting_function --> impulse_time_weighting_behavior : configures_for_impulse
class initial_state_handling {
+initial_state : str|float|np.ndarray|None
+state_shape : tuple
+state : np.ndarray
}
prepare_time_weighting_initial_state_function --> initial_state_handling : implements_rules
class setuptools_dynamic_version {
+pyproject_dynamic_version
}
setuptools_dynamic_version --> _version_module : reads_attr
setuptools_dynamic_version --> package_init_module : aligned_with
File-Level Changes
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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughPackage version management is centralized from inline constants to a dedicated ChangesVersion Management & Time-weighting Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/pyoctaveband/parametric_filters.py" line_range="233-234" />
<code_context>
a = [1, -(1 - alpha)]
# We apply the weighting to the squared signal to get the Mean Square value
- return cast(np.ndarray, signal.lfilter(b, a, x_sq, axis=-1))
+ zi = np.expand_dims((1 - alpha) * initial, axis=-1)
+ y, _ = signal.lfilter(b, a, x_sq, axis=-1, zi=zi)
+ return cast(np.ndarray, y)
</code_context>
<issue_to_address>
**issue (bug_risk):** The `zi` shape for `signal.lfilter` is likely incorrect for multi-dimensional inputs and may lead to misaligned state or runtime errors.
For `lfilter`, `zi` must have shape `(max(len(a), len(b)) - 1, ...)`, with the leading dimension for filter state and the remaining dimensions matching `x` with the filtering axis removed. With `x_sq` shaped `(*state_shape, time)` and `axis=-1`, and a first-order IIR (`len(a) == 2`, `len(b) == 1`), `zi` should be `(1, *state_shape)`. Using `np.expand_dims(..., axis=-1)` instead yields `(*state_shape, 1)`, putting the state dimension in the wrong position for multi-dimensional inputs. Please construct `zi` as `((1 - alpha) * initial)[None, ...]` (or equivalent) so the state dimension is leading and the remaining dimensions align with `x_sq` minus the time axis.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+233
to
+234
| zi = np.expand_dims((1 - alpha) * initial, axis=-1) | ||
| y, _ = signal.lfilter(b, a, x_sq, axis=-1, zi=zi) |
There was a problem hiding this comment.
issue (bug_risk): The zi shape for signal.lfilter is likely incorrect for multi-dimensional inputs and may lead to misaligned state or runtime errors.
For lfilter, zi must have shape (max(len(a), len(b)) - 1, ...), with the leading dimension for filter state and the remaining dimensions matching x with the filtering axis removed. With x_sq shaped (*state_shape, time) and axis=-1, and a first-order IIR (len(a) == 2, len(b) == 1), zi should be (1, *state_shape). Using np.expand_dims(..., axis=-1) instead yields (*state_shape, 1), putting the state dimension in the wrong position for multi-dimensional inputs. Please construct zi as ((1 - alpha) * initial)[None, ...] (or equivalent) so the state dimension is leading and the remaining dimensions align with x_sq minus the time axis.
![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
This pull request introduces an initial_state parameter to the time_weighting function, enabling support for block processing and custom initialization of the exponential integrator. Additionally, the package versioning has been moved to a dynamic configuration, and the test suite was significantly restructured and expanded to improve coverage. A high-severity issue was identified regarding the shape of initial conditions for multichannel signals in time_weighting, which would cause a runtime error.
| a = [1, -(1 - alpha)] | ||
| # We apply the weighting to the squared signal to get the Mean Square value | ||
| return cast(np.ndarray, signal.lfilter(b, a, x_sq, axis=-1)) | ||
| zi = np.expand_dims((1 - alpha) * initial, axis=-1) |
There was a problem hiding this comment.
The initial conditions zi for scipy.signal.lfilter are incorrectly shaped for multichannel signals. When filtering along the last dimension (axis=-1), lfilter expects zi to have the shape (order, *remaining_dims). Currently, np.expand_dims(..., axis=-1) results in (*remaining_dims, order), which will cause a ValueError during execution for any input with more than one channel (e.g., stereo audio). Changing the axis to 0 ensures the order dimension is correctly placed at the beginning.
Suggested change
| zi = np.expand_dims((1 - alpha) * initial, axis=-1) | |
| zi = np.expand_dims((1 - alpha) * initial, axis=0) |
Contributor
CI Results 🚀Test Summary
Technical Benchmark Summary📊 View Benchmark DetailsPyOctaveBand: Technical Benchmark ReportGenerated: 2026-05-09 12:33:17 1. Test Signal Parameters
2. Crossover (Linkwitz-Riley)
3. Precision & Isolation
4. Performance
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@README.md`:
- Line 93: Update the README entry for time_weighting to clarify that the
returned envelope "env" preserves the input shape (i.e., is aligned with x and
supports multichannel inputs), not always a 1D array; mention it returns an
array with the same number of channels as x (or same shape as x) and give the
existing call signature time_weighting(x, fs, mode='fast') and symbol env so
readers know which function and return value you mean.
In `@src/pyoctaveband/parametric_filters.py`:
- Around line 221-223: Guard against non-positive sampling rates by validating
the fs parameter at the start of the function that uses it (the function that
calls _typesignal, computes x_sq and calls
_prepare_time_weighting_initial_state). If fs <= 0 raise a ValueError with a
clear message, or return an appropriate error path consistent with other module
APIs; do this before any computation of integration/constants that depend on fs
so downstream math (e.g., integration constants, filter coefficients) never sees
invalid values. Update the function containing x_proc = _typesignal(x), x_sq =
x_proc**2, initial = _prepare_time_weighting_initial_state(...) to perform this
check early.
- Around line 166-168: The branch handling initial_state == "first" currently
assumes x_sq has at least one sample and will IndexError on empty input; update
the block (the code referencing state_name and x_sq and using np.take(..., 0,
axis=-1)) to first check whether the input has any samples along the last axis
(e.g., if x_sq.shape[-1] == 0 or x_sq.size == 0) and if empty raise the same
ValueError("initial_state must be None, 'zero', 'first', a scalar, or an array")
instead of indexing; otherwise proceed to return the first-sample array as
before.
In `@tests/test_utils.py`:
- Around line 11-27: The test uses plain np.ones so it can miss
ordering/selection bugs; replace the inputs with distinct sequences (e.g.,
np.arange or row-wise ranges) and assert exact value preservation/trimming and
zero padding. For padding cases (calls to _resample_to_length with target 12),
assert y[:10] equals the original sequence and y[10:] are zeros; for the 2D case
assert each row's first 10 columns equal the corresponding input row and columns
10: are zeros. For trimming cases (target 8), assert y equals the input's first
8 samples (and for 2D, first 8 columns per row) to ensure correct order and
content are preserved.
🪄 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: CHILL
Plan: Pro
Run ID: 634d7fd5-1d66-49da-966a-b857004ae0c7
📒 Files selected for processing (13)
.github/workflows/release.ymlREADME.mdpyproject.tomlsrc/pyoctaveband/__init__.pysrc/pyoctaveband/_version.pysrc/pyoctaveband/parametric_filters.pytests/test_basic.pytests/test_coverage_fix.pytests/test_errors_and_edge_cases.pytests/test_filter_design.pytests/test_nominal_frequencies.pytests/test_parametric_filters.pytests/test_utils.py
💤 Files with no reviewable changes (1)
- tests/test_coverage_fix.py
Contributor
CI Results 🚀Test Summary
Technical Benchmark Summary📊 View Benchmark DetailsPyOctaveBand: Technical Benchmark ReportGenerated: 2026-05-09 12:43:33 1. Test Signal Parameters
2. Crossover (Linkwitz-Riley)
3. Precision & Isolation
4. Performance
|
Contributor
CI Results 🚀Test Summary
Technical Benchmark Summary📊 View Benchmark DetailsPyOctaveBand: Technical Benchmark ReportGenerated: 2026-05-09 12:46:25 1. Test Signal Parameters
2. Crossover (Linkwitz-Riley)
3. Precision & Isolation
4. Performance
|
Contributor
CI Results 🚀Test Summary
Technical Benchmark Summary📊 View Benchmark DetailsPyOctaveBand: Technical Benchmark ReportGenerated: 2026-05-09 12:50:38 1. Test Signal Parameters
2. Crossover (Linkwitz-Riley)
3. Precision & Isolation
4. Performance
|
Contributor
CI Results 🚀Test Summary
Technical Benchmark Summary📊 View Benchmark DetailsPyOctaveBand: Technical Benchmark ReportGenerated: 2026-05-09 12:51:59 1. Test Signal Parameters
2. Crossover (Linkwitz-Riley)
3. Precision & Isolation
4. Performance
|
Contributor
CI Results 🚀Test Summary
Technical Benchmark Summary📊 View Benchmark DetailsPyOctaveBand: Technical Benchmark ReportGenerated: 2026-05-09 12:55:22 1. Test Signal Parameters
2. Crossover (Linkwitz-Riley)
3. Precision & Isolation
4. Performance
|
|
Contributor
CI Results 🚀Test Summary
Technical Benchmark Summary📊 View Benchmark DetailsPyOctaveBand: Technical Benchmark ReportGenerated: 2026-05-09 12:59:32 1. Test Signal Parameters
2. Crossover (Linkwitz-Riley)
3. Precision & Isolation
4. Performance
|
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.
Adds optional initial-state control to
time_weightingand prepares the package for the next tagged release.Summary:
initial_statesupport for FAST/SLOW and impulse time weighting while preserving the existing zero-state default.src/pyoctaveband/_version.pyand bump it to1.2.2.v*tags instead of pushes tomain, validating that the tag matches the package version.test_coverage_fix.pycases into semantic test modules.Validation:
python -m build --outdir <tmpdir>python -m twine check <tmpdir>/*python -m pytest tests/(117 passed, one existing detrending warning)python -m ruff check .python -m mypy srcactionlint .github/workflows/release.ymlgit diff --checkRefs #38
Summary by Sourcery
Add configurable initial state handling for time weighting, centralize and expose the package version, and adjust release and tests to support the new behavior and coverage.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Summary by CodeRabbit
New Features
initial_stateparameter to time weighting for flexible exponential integrator initialization across signal segmentsDocumentation
Chores