Skip to content

[Refactor] Remove resampy dependency#2891

Merged
Isotr0py merged 1 commit intovllm-project:mainfrom
NickCao:resampy
Apr 18, 2026
Merged

[Refactor] Remove resampy dependency#2891
Isotr0py merged 1 commit intovllm-project:mainfrom
NickCao:resampy

Conversation

@NickCao
Copy link
Copy Markdown
Contributor

@NickCao NickCao commented Apr 17, 2026

Purpose

Follows vllm-project/vllm#39524

Test Plan

TBD

Test Result

TBD


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan. Please provide the test scripts & test commands. Please state the reasons if your codes don't require additional test scripts. For test file guidelines, please check the test style doc
  • The test results. Please paste the results comparison before and after, or the e2e results.
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model. Please run mkdocs serve to sync the documentation editions to ./docs.
  • (Optional) Release notes update. If your change is user-facing, please update the release notes draft.

BEFORE SUBMITTING, PLEASE READ https://github.com/vllm-project/vllm-omni/blob/main/CONTRIBUTING.md (anything written below this line will be removed by GitHub Actions)

Signed-off-by: Nick Cao <ncao@redhat.com>
@NickCao NickCao requested a review from hsliuustc0106 as a code owner April 17, 2026 17:21
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@NickCao
Copy link
Copy Markdown
Contributor Author

NickCao commented Apr 17, 2026

cc @Isotr0py

@hsliuustc0106
Copy link
Copy Markdown
Collaborator

BLOCKER scan:

  • Correctness: PASS (API replacement looks correct)
  • Reliability/Safety: PASS
  • Breaking Changes: Needs clarification - is removing resampy from requirements/common.txt a user-facing change?
  • Test Coverage: ISSUE - Test plan is TBD. Need to verify audio resampling works correctly with AudioResampler
  • Documentation: ISSUE - No documentation update about the dependency removal
  • Security: PASS

BLOCKING ISSUES:

  1. Test Coverage - Missing test plan and results. Please verify that audio resampling produces the same output with AudioResampler as with resampy
  2. Documentation - Should document the dependency removal and any migration steps if users were using resampy directly
  3. Performance consideration - Creating a new AudioResampler instance each time (in loops) may be less efficient than the stateless resample_audio_resampy. Consider if this needs to be optimized

VERDICT: REQUEST_CHANGES


Note: Since this is removing a dependency, please also verify that AudioResampler is available in the minimum supported vLLM version.

@Isotr0py Isotr0py enabled auto-merge (squash) April 18, 2026 02:55
@Isotr0py Isotr0py added the ready label to trigger buildkite CI label Apr 18, 2026
@Isotr0py Isotr0py merged commit d2c23d7 into vllm-project:main Apr 18, 2026
7 of 8 checks passed
lvliang-intel pushed a commit to lvliang-intel/vllm-omni that referenced this pull request Apr 20, 2026
Signed-off-by: Nick Cao <ncao@redhat.com>
qinganrice pushed a commit to qinganrice/vllm-omni that referenced this pull request Apr 23, 2026
Signed-off-by: Nick Cao <ncao@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants