Removed custom_output_names from the separate file call.#260
Removed custom_output_names from the separate file call.#260Kulin-Soni wants to merge 2 commits into
Conversation
Removed custom_output_names from _separate_file call inside the _process_with_chunking method. This prevents the processed chunks from getting the same custom name, and rather have the `chunk_N` naming.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughRemoved the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use your project's `ruff` configuration to improve the quality of Python code reviews.Add a Ruff configuration file to your project to customize how CodeRabbit runs |
|
Can you make sure all the tests pass too please? 🙏 |
|
sure, will do soon |
|
@beveradb Tests done! |
… defaults Fix chunking bug where custom_output_names passed to _separate_file caused chunks to overwrite each other (fixes #259). Custom names are now only applied to the final merged output. Adds regression test with multi-chunk + multi-stem scenario that verifies _separate_file is called without custom names. Also clarifies demucs_params segment_size documentation: "Default" maps to 40 for older Demucs models and 10 for Demucs v4/htdemucs. Incorporates work from PR #260 (@Kulin-Soni) and PR #264 (@Madduri-Ganesh) with additional test coverage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… defaults (#270) Fix chunking bug where custom_output_names passed to _separate_file caused chunks to overwrite each other (fixes #259). Custom names are now only applied to the final merged output. Adds regression test with multi-chunk + multi-stem scenario that verifies _separate_file is called without custom names. Also clarifies demucs_params segment_size documentation: "Default" maps to 40 for older Demucs models and 10 for Demucs v4/htdemucs. Incorporates work from PR #260 (@Kulin-Soni) and PR #264 (@Madduri-Ganesh) with additional test coverage. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Hey @Kulin-Soni — thank you so much for identifying and fixing this bug! Your analysis of the issue was spot-on: passing I've incorporated your fix into #270 (now merged), which also adds a more comprehensive regression test that uses multiple chunks + multiple stems and explicitly verifies that I'm going to close this PR as superseded, but want to make sure you know your contribution was the foundation of the fix. Really appreciate you taking the time to track this down and submit a PR! 🙏 Merged in: #270 |
|
These changes will be available in version 0.42.1 (release PR: #271). |
Removed custom_output_names from _separate_file call inside the _process_with_chunking method. This prevents the processed chunks from getting the same custom name, and have the
chunk_Nnaming.This prevents overwriting previous chunks.
This is a fix to #259
Summary by CodeRabbit