Improve API and make streaming responses possible #71
Conversation
Neverbolt
force-pushed
the
openai-streaming
branch
from
88ebdf0 to
39327ef
Compare
There was a problem hiding this comment.
would it be possible to add a unit test with a simpe example capability?
There was a problem hiding this comment.
do you need it on this MR?
There was a problem hiding this comment.
I think it's okay for this merge reqeust (and the existing unit tests are passing so I kinda hoping that it will not break something). You can create a follow-up merge request with a couple of unit tests
| @@ -31,32 +40,116 @@ def client(self) -> openai.OpenAI: | |||
| def instructor(self) -> instructor.Instructor: | |||
| return instructor.from_openai(self.client) | |||
|
|
|||
| def get_response(self, prompt, *, capabilities=None, **kwargs) -> LLMResult: | |||
| def get_response(self, prompt, *, capabilities: Dict[str, Capability]=None, **kwargs) -> LLMResult: | |||
There was a problem hiding this comment.
type information for prompt missing
There was a problem hiding this comment.
the parent class also has no type information for prompt, what should we limit it to? keep in mind the currently commented out code below that is a compatibility layer to allow both string, renderable and full message list support
There was a problem hiding this comment.
what would you suggest? I think the code was written by you (:
There was a problem hiding this comment.
well there was a reason for not adding a type, so that we can vary in the implementations :)
|
So this:
|
No description provided.