fix: resolve python pytest warnings and deprecations - #155
Conversation
…fix/issue-153-warnings-deprecations
|
Warning Review limit reached
More reviews will be available in 2 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py`:
- Around line 39-40: Replace the Path(audio_path).exists() check with
Path(audio_path).is_file() to ensure only regular files (not directories or
special files) are accepted at the input boundary; keep raising
FileNotFoundError when the check fails but update the error message to reference
the actual input variable (audio_path) instead of path_str, and leave downstream
callers (e.g., librosa.load) to assume a verified file path.
- Around line 46-49: The current warnings.catch_warnings() block around the
librosa.load call ignores all DeprecationWarning/FutureWarning globally; narrow
this to only suppress warnings coming from audioread by replacing the
simplefilter calls with warnings.filterwarnings("ignore",
category=DeprecationWarning, module=r"^audioread") and
warnings.filterwarnings("ignore", category=FutureWarning, module=r"^audioread")
inside the same with warnings.catch_warnings(): context that surrounds the y, sr
= librosa.load(path_str, sr=TARGET_SR, mono=True) call so other
deprecation/future warnings still surface.
In `@services/analysis-engine/tests/test_chord_recognizer.py`:
- Line 50: Multiple tests create signals using repeated expressions like "y =
np.random.randn(22050 * 3)"; extract a shared constant or helper to compute
sample counts (e.g., SAMPLE_RATE = 22050 and DURATION_SECONDS = 3 or a helper
samples(seconds)) and replace all occurrences in the tests (references: variable
y initializations across test_chord_recognizer tests and any other tests that
build random signals) to use SAMPLE_RATE * DURATION_SECONDS or samples(3).
Update test setup/fixtures to import or define these constants/helpers so future
changes to sample rate or duration only require one edit.
🪄 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
Run ID: 5e9504e5-4480-4305-a01b-81504cac8dc1
📒 Files selected for processing (3)
services/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyservices/analysis-engine/tests/test_chord_recognizer.pyservices/analysis-engine/tests/test_temporal.py
64acaf2 to
d297ba8
Compare
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
Summary
Resolves #154. This PR cleans up the pytest environment in
services/analysis-engineso it runs with zero warnings:RuntimeWarningforrunpy.run_moduleintest_cli.py.n_fft=1024 is too large for input signalwarnings intest_chord_recognizer.pyby generating 3.0-second synthetic signals instead of 1.0-second signals.aifc,audioop,sunaudeprecation warnings andaudioreadfallbacks intest_temporal.pyby safely catching them and verifying file existence upfront.Verification
export UV_PYTHON=3.12 && uv run pytest tests --cov=src/bandscope_analysis -W errorpasses successfully with 100% coverage and zero warnings.quickcheck.sh) passes.