Skip to content

Conversation

@jordanxlau
Copy link
Collaborator

@jordanxlau jordanxlau commented Aug 11, 2025

PR Goal?

Allow the user to configure text chunking boundaries in TextConfig (eg. adding an interrobang or ellipsis to the list of strong boundaries, or a dash to the list of weak boundaries).

For more information see: EveryVoiceTTS/EveryVoice#721.

Tests added?

None in submodule - I believe existing tests in main module cover everything.

Related PRs?

See parent PR: See EveryVoiceTTS/EveryVoice#721

@semanticdiff-com
Copy link

semanticdiff-com bot commented Aug 11, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  fs2/tests/test_cli.py  72% smaller
  fs2/cli/synthesize.py  42% smaller

@jordanxlau jordanxlau marked this pull request as draft August 11, 2025 16:33
@codecov
Copy link

codecov bot commented Aug 12, 2025

Codecov Report

❌ Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.59%. Comparing base (4d453d3) to head (9f04b42).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
fs2/cli/synthesize.py 92.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #125      +/-   ##
==========================================
+ Coverage   49.17%   49.59%   +0.41%     
==========================================
  Files          27       27              
  Lines        1948     1968      +20     
==========================================
+ Hits          958      976      +18     
- Misses        990      992       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jordanxlau jordanxlau force-pushed the dev.jl/configure_boundaries branch 4 times, most recently from e248df1 to 9a7134a Compare August 14, 2025 16:43
@jordanxlau jordanxlau requested a review from roedoejet August 14, 2025 16:49
@jordanxlau jordanxlau marked this pull request as ready for review August 14, 2025 16:50
Copy link
Member

@roedoejet roedoejet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple clarifying questions, but it looks good to me - thanks @jordanxlau !


text_config: TextConfig = model.config.text
split_text: bool = text_config.split_text
strong: str = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
strong: str = ""
strong_boundaries: str = ""

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or maybe even strong_textsplit_boundaries

text_config: TextConfig = model.config.text
split_text: bool = text_config.split_text
strong: str = ""
weak: str = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
weak: str = ""
weak_boundaries: str = ""


text_config: TextConfig = model.config.text
split_text: bool = text_config.split_text
strong: str = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here - we could be a bit more clear in the var name

# Checks that the files have reasonable lengths
output_one = AudioSegment.from_file(
output_dir / "one--S1--L1--ckpt=77--v_ckpt=10--pred.wav"
output_dir / "one--S1--L1--ckpt=77--v_ckpt=10--pred.wav", close=True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see close in the documentation - what is this for? https://github.com/jiaaro/pydub/blob/master/API.markdown

@jordanxlau jordanxlau force-pushed the dev.jl/configure_boundaries branch from 9a7134a to 52a7801 Compare August 15, 2025 22:05
@jordanxlau jordanxlau force-pushed the dev.jl/configure_boundaries branch from 52a7801 to 2412d66 Compare August 15, 2025 22:30
@jordanxlau jordanxlau force-pushed the dev.jl/configure_boundaries branch from 2412d66 to 9f04b42 Compare August 15, 2025 22:30
@jordanxlau jordanxlau requested a review from roedoejet August 18, 2025 18:22
Copy link
Member

@roedoejet roedoejet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - thanks @jordanxlau !

@jordanxlau jordanxlau merged commit 9f04b42 into main Aug 18, 2025
7 checks passed
@jordanxlau jordanxlau deleted the dev.jl/configure_boundaries branch August 18, 2025 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants