Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update_recipes #1208

Merged
merged 2 commits into from
Nov 10, 2023
Merged

update_recipes #1208

merged 2 commits into from
Nov 10, 2023

Conversation

KarelVesely84
Copy link
Contributor

Proposing small changes to the recipes:

  • commonvoice : use "spawn" context in ProcessPoolExecutor
  • eval2000 : bugfix (@click.option must begin with --*)
  • librispeech : add --to-lowercase for librispeech dataprep (default are uppercase transcripts)

- commonvoice : use "spawn" context in `ProcessPoolExecutor`
- eval2000 : bugfix (@click.option must begin with `--*`)
- librispeech : add `--to-lowercase` for librispeech dataprep (default are uppercase transcripts)
Comment on lines 37 to 42
@click.option(
"--to-lowercase",
type=bool,
default=False,
help="Conversion of transcripts to lower-vase (originally in uppercase).",
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@vesis84 most other recipes have an option called --normalize-text with choices such as "upper", "kaldi", etc. Maybe you can name this option the same for consistency?

@KarelVesely84 KarelVesely84 force-pushed the update_recipes branch 2 times, most recently from b8124a4 to ed03ca7 Compare November 7, 2023 11:10
@KarelVesely84
Copy link
Contributor Author

KarelVesely84 commented Nov 7, 2023

Hi, thanks @desh2608 for the suggestion. Now, it is refactored accordingly ( and tested ).
Best, Karel

"--normalize-text",
type=click.Choice(["none", "lower"], case_sensitive=False),
default="none",
help="Conversion of transcripts to lower-vase (originally in uppercase).",
Copy link
Collaborator

Choose a reason for hiding this comment

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

lower-vase --> lower-case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, corrected now...

Copy link
Collaborator

@pzelasko pzelasko left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@pzelasko pzelasko merged commit 3d1f4b5 into lhotse-speech:master Nov 10, 2023
10 checks passed
@pzelasko pzelasko added this to the v1.18 milestone Nov 10, 2023
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.

None yet

3 participants