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

17 implement cli based interaction #34

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

daavoo
Copy link
Contributor

@daavoo daavoo commented Dec 3, 2024

What's changing

  • Create cli.py and expose it as entrypoint.
  • Create config and refactor how speaker configuration is handled.
  • Update demo to only expose speaker customization instead of the full prompt.
    image
  • Update unit tests to use mock.
  • Add e2e tests that can be run through workflow_dispatch (takes too much time to be enabled by default)

How to test it

  • CLI
document-to-podcast --from_config example_data/config.yaml
  • Demo change:
python -m streamlit run demo/app.py
  • Docs updates:
mkdocs serve

@daavoo daavoo self-assigned this Dec 3, 2024
@daavoo daavoo linked an issue Dec 3, 2024 that may be closed by this pull request
@daavoo daavoo marked this pull request as draft December 4, 2024 15:08
@daavoo daavoo force-pushed the 17-implement-cli-based-interaction branch from 4e95724 to 1bded51 Compare December 5, 2024 10:22
@stefanfrench
Copy link
Contributor

@daavoo - Did an early test of this and its working well - tested on codespaces and worked end-to-end with the output saving. thanks for also updating the docs.

@daavoo daavoo force-pushed the 17-implement-cli-based-interaction branch from bd56198 to caae572 Compare December 10, 2024 12:27
@daavoo daavoo mentioned this pull request Dec 11, 2024
3 tasks
@daavoo daavoo force-pushed the 17-implement-cli-based-interaction branch from fe55a78 to bf5698b Compare December 11, 2024 11:10
@daavoo daavoo marked this pull request as ready for review December 11, 2024 11:10
@daavoo daavoo requested review from Kostis-S-Z, stefanfrench and a team and removed request for Kostis-S-Z and stefanfrench December 11, 2024 11:10
@stefanfrench
Copy link
Contributor

@daavoo - Thanks for this, a lot of work here!

  • Testing the CLI was smooth, no issues found and yaml is clear.
  • I was testing the new streamlit app locally. I really like the speaker configuration, however after generating the podcast, I got this error:
    image

@daavoo
Copy link
Contributor Author

daavoo commented Dec 11, 2024

  • however after generating the podcast, I got this error:

Taking a look 👀

@daavoo
Copy link
Contributor Author

daavoo commented Dec 11, 2024

  • however after generating the podcast, I got this error:

Taking a look 👀

@stefanfrench I think this is fixed now

Copy link
Contributor

@stefanfrench stefanfrench left a comment

Choose a reason for hiding this comment

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

  • Tested CLI works
  • Tested updated streamlit app works
  • Tested mkdocs serve

All working as expected, approved for merge. Thanks for the big effort on this. I think we will need to update the customiozation guide docs based on the changes, but we can do that in a separate issue.

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.

Implement CLI based interaction
2 participants