-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
AnswerToSpeech
#2584
AnswerToSpeech
#2584
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very promising! I left a few comments that we could discuss and I would like to see a small example pipeline/tutorial. Before we can merge this we would need to add docstrings and also test cases so there is still some work to do but so far it looks great! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 👍 This is still a bit experimental I'd say and our documentation could mention that. It will be good for us to see how the community uses this feature. We could even ask for feedback on Haystack slack explicitly. Maybe a community member wants to showcase a use case, who knows? Let's make it as simple as possible for the community to try this out. I left a few comments regarding doc strings and I'll add the needs-documentation to make @agnieszka-m aware of this upcoming new pipeline node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 👍
Proposed changes:
AudioAnswer
andGeneratedAudioAnswer
, that contains references to audio files into theiranswer
andcontext
fields.AnswerToSpeech
that can convert regular Answers into aGeneratedAudioAnswer
.Open questions:
Status (please check what you already did):