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

Minor fixes to ds_tool and infer_tool #36

Merged
merged 3 commits into from
Jun 24, 2024
Merged

Minor fixes to ds_tool and infer_tool #36

merged 3 commits into from
Jun 24, 2024

Conversation

juberti
Copy link
Contributor

@juberti juberti commented Jun 23, 2024

  • --upload_split param to allow the dest split to be different than the src split
  • allow @file syntax for --prompt
  • add retries and timeouts to TTS requests

- --upload_split param to allow the dest split to be different than the src split
- allow @file syntax for --prompt
- add retries and timeouts to TTS requests
@juberti juberti requested a review from farzadab June 23, 2024 01:59
ultravox/tools/ds_tool.py Outdated Show resolved Hide resolved
@juberti juberti merged commit 66dd04f into main Jun 24, 2024
1 check passed
@@ -150,6 +159,7 @@ def main(args: DatasetToolArgs):
"token": token,
"revision": args.upload_branch,
"private": args.private,
"split": args.upload_split,
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, you didn't fix the conflict here!
Now on line 170 you'll be sending the split argument twice, and with different values!

Comment on lines +134 to +136
assert (
not self.upload_split or self.dataset_split
), "Must specify dataset_split when using upload_split"
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I didn't catch this the first time, but why can't upload_split be equal to dataset_split by default if not specified?

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.

2 participants