fix: resolve python pytest warnings and deprecations#155
Open
seonghobae wants to merge 5 commits into
Open
Conversation
…fix/issue-153-warnings-deprecations
📝 WalkthroughSummary by CodeRabbit릴리스 노트
Walkthrough
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 docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
Comment on lines
+39
to
+40
| if not Path(audio_path).exists(): | ||
| raise FileNotFoundError(f"Audio file not found: {path_str}") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 목적: 파일 경계 검증 방식 점검 (exists vs is_file)
rg -n --type=py 'Path\(audio_path\)\.exists\(|\.is_file\(' services/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyRepository: seonghobae/bandscope
Length of output: 109
🏁 Script executed:
cat -n services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py | head -60Repository: seonghobae/bandscope
Length of output: 2405
🏁 Script executed:
# Check the full function context around lines 39-40
sed -n '25,55p' services/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyRepository: seonghobae/bandscope
Length of output: 1230
🏁 Script executed:
# Check if there are type hints or documentation
sed -n '1,50p' services/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyRepository: seonghobae/bandscope
Length of output: 1499
🏁 Script executed:
# Check if there are similar validation patterns elsewhere in the file
rg -n 'Path.*\.(exists|is_file)\(' services/analysis-engine/src/bandscope_analysis/Repository: seonghobae/bandscope
Length of output: 178
🏁 Script executed:
# Check if librosa.load is used elsewhere to see how errors are typically handled
rg -n 'librosa\.load' services/analysis-engine/src/bandscope_analysis/Repository: seonghobae/bandscope
Length of output: 355
입력 경계에서 일반 파일 여부를 검증하도록 수정하세요.
Path.exists()는 디렉터리와 특수 파일도 True를 반환합니다. 현재 코드는 디렉터리를 통과시키면 librosa.load()에서 다른 예외가 발생하게 됩니다. 문서에서 "Path to the audio file"로 명시된 대로 입력 경계에서 정확히 일반 파일만 허용하도록 is_file()을 사용하세요.
제안 수정
- path_str = str(audio_path)
- if not Path(audio_path).exists():
+ path = Path(audio_path)
+ path_str = str(path)
+ if not path.is_file():
raise FileNotFoundError(f"Audio file not found: {path_str}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py` around
lines 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.
Comment on lines
+46
to
+49
| with warnings.catch_warnings(): | ||
| warnings.simplefilter("ignore", category=DeprecationWarning) | ||
| warnings.simplefilter("ignore", category=FutureWarning) | ||
| y, sr = librosa.load(path_str, sr=TARGET_SR, mono=True) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 목적: 런타임 코드의 광범위 ignore 필터 존재 여부 확인
rg -n --type=py 'simplefilter\("ignore", category=(DeprecationWarning|FutureWarning)\)|filterwarnings\("ignore"' services/analysis-engine/srcRepository: seonghobae/bandscope
Length of output: 357
🏁 Script executed:
# 1. 전체 코드베이스에서 모든 경고 관련 필터/무시 패턴 검색
rg -n --type=py 'warnings\.(simplefilter|filterwarnings|catch_warnings)' services/analysis-engine/src
# 2. librosa 로드 주변의 전체 함수 컨텍스트 확인
rg -B 15 -A 5 'librosa\.load' services/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyRepository: seonghobae/bandscope
Length of output: 1495
🏁 Script executed:
# 1. audioread이 프로젝트에서 사용되는지 확인
rg -n 'audioread' services/analysis-engine/src
# 2. 프로젝트 의존성 확인 (requirements, setup.py, pyproject.toml 등)
fd -t f '(requirements|setup|pyproject|poetry|pipfile)' services/analysis-engine/ | head -20
# 3. librosa 사용 패턴 확인
rg -n 'import librosa' services/analysis-engine/src
# 4. 코드에 경고 무시 이유에 대한 주석이 있는지 확인
rg -B 5 'catch_warnings' services/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyRepository: seonghobae/bandscope
Length of output: 569
🏁 Script executed:
# pyproject.toml 내용 확인
cat services/analysis-engine/pyproject.toml | head -100Repository: seonghobae/bandscope
Length of output: 850
librosa.load() 호출 시 특정 모듈로 경고 필터링 범위 좁히기
DeprecationWarning/FutureWarning 전체를 무시하면 라이브러리 업그레이드 시 호환성 문제를 조기에 탐지하기 어렵습니다. audioread(librosa의 음성 로드 백엔드)에서만 이 경고들을 무시하도록 범위를 좁혀주세요.
제안 수정
with warnings.catch_warnings():
- warnings.simplefilter("ignore", category=DeprecationWarning)
- warnings.simplefilter("ignore", category=FutureWarning)
+ warnings.filterwarnings(
+ "ignore",
+ category=DeprecationWarning,
+ module=r"audioread\..*",
+ )
+ warnings.filterwarnings(
+ "ignore",
+ category=FutureWarning,
+ module=r"audioread\..*",
+ )
y, sr = librosa.load(path_str, sr=TARGET_SR, mono=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py` around
lines 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.
| """Test for test_chord_recognizer_hpss_exception.""" | ||
| recognizer = ChordRecognizer() | ||
| y = np.random.randn(22050) | ||
| y = np.random.randn(22050 * 3) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
반복되는 샘플 수 계산은 상수/헬퍼로 통일하는 것이 좋습니다.
현재 패턴이 여러 테스트에 반복되어, 샘플링 레이트나 길이 변경 시 수정 지점이 많아집니다. 공통 상수/헬퍼로 추출하면 유지보수성이 좋아집니다.
예시 리팩터링 diff
import numpy as np
from bandscope_analysis.chords.chord_recognizer import ChordRecognizer
+SR = 22050
+DURATION_SEC = 3
+
+
+def _noise_signal(sr: int = SR, duration_sec: int = DURATION_SEC) -> np.ndarray:
+ return np.random.randn(sr * duration_sec)
@@
def test_chord_recognizer_hpss_exception():
@@
- y = np.random.randn(22050 * 3)
+ y = _noise_signal()
@@
def test_chord_recognizer_chroma_cqt_exception():
@@
- y = np.random.randn(22050 * 3)
+ y = _noise_signal()
@@
def test_chord_recognizer_rms_exception():
@@
- y = np.random.randn(22050 * 3)
+ y = _noise_signal()
@@
def test_chord_recognizer_rms_padding():
@@
- y = np.random.randn(22050 * 3)
+ y = _noise_signal()
@@
def test_chord_recognizer_empty_chromagram():
@@
- y = np.random.randn(22050 * 3)
+ y = _noise_signal()
@@
def test_chord_recognizer_rms_longer():
@@
- y = np.random.randn(22050 * 3)
+ y = _noise_signal()Also applies to: 60-60, 70-70, 80-80, 94-94, 105-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/analysis-engine/tests/test_chord_recognizer.py` at 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.
This was referenced Apr 29, 2026
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
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.