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

feat: Change CLI backend to Typer #19

Merged
merged 4 commits into from
Jan 21, 2024

Conversation

abhijeetSaroha
Copy link
Contributor

Change CLI backend to typer.

Solves #12

@abhijeetSaroha
Copy link
Contributor Author

The error in text_to_speech() comes from file tts.py at the time of saving the output.

All other commands work fine, as I have checked all except text_to_speech().
image

@xmnlab xmnlab changed the title WIP: change cli backend to typer feat: Change CLI backend to Typer Jan 21, 2024
@xmnlab
Copy link
Member

xmnlab commented Jan 21, 2024

@abhijeetSaroha I just fixed some issues on upstream/main .. and rebased your branch

@xmnlab
Copy link
Member

xmnlab commented Jan 21, 2024

I am going to play with your new implementation now

@xmnlab
Copy link
Member

xmnlab commented Jan 21, 2024

@abhijeetSaroha , playing with that I found just few issus that would fix your code here.

this is the diff from changes:

diff --git a/src/artbox/cli.py b/src/artbox/cli.py
index 69dde42..247624d 100644
--- a/src/artbox/cli.py
+++ b/src/artbox/cli.py
@@ -92,13 +92,13 @@ def text_to_speech(
         typer.Option(
             "--lang", help="Choose the language for audio generation"
         ),
-    ] = "",
+    ] = "en",
 ) -> None:
     """Convert text to speech."""
     args_dict = {
         "title": title,
         "text-path": text_path,
-        "output_path": output_path,
+        "output-path": output_path,
         "engine": engine,
         "lang": lang,
     }

after that, check if that fixes the issues you had locally as well, please.

@abhijeetSaroha
Copy link
Contributor Author

@xmnlab , corrected that mistake.

typer.Option(
"--lang", help="Choose the language for audio generation"
),
] = "",
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
] = "",
] = "en",

it is also necessary to define its default as "en"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@xmnlab
Copy link
Member

xmnlab commented Jan 21, 2024

LGTM! thanks for working on that @abhijeetSaroha ! appreciate that. good job here.
waiting the CI to be green and I will merge this PR. thanks!

@xmnlab xmnlab merged commit 8223bd1 into osl-incubator:main Jan 21, 2024
7 checks passed
Copy link

🎉 This PR is included in version 0.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants