enhancement: Reduce reliance on docs by expanding kwargs into parameters #7
Conversation
|
That's my speciality 😉 I'll extend it to the other classes too! Let me tackle another PR first. |
| model: Optional[str] = None, | ||
| temperature: float = 0.9, | ||
| top_p: float = 0.6, | ||
| top_k: Optional[int] = 10, |
There was a problem hiding this comment.
In this implementation, top_k can technically be None, but it might be best to simply turn it into top_k: int = 10, as you wouldn't really want users to specify None. Same for frequency_penalty.
There was a problem hiding this comment.
TGI supports none for both and then uses 0 or omits the values basically.
There was a problem hiding this comment.
It does, but EasyLLM doesn't:
easyllm/easyllm/clients/huggingface.py
Line 148 in 00ec957
I can fix this though. I'll introduce the possibility for None.
|
Could you quickly rebase your PR? i merged the fix for |
Some ~5 arguments were listed as Optional[X], which is equivalent to Union[X, None]. However, None caused crashes for these parameters.
and slightly update the docstrings
tomaarsen
force-pushed
the
enhancement/expand_kwargs
branch
from
19d5b3d
to
49cd588
Compare
| model: Optional[str] = None, | ||
| temperature: float = 0.9, | ||
| top_p: float = 0.6, | ||
| top_k: Optional[int] = 10, |
There was a problem hiding this comment.
TGI supports none for both and then uses 0 or omits the values basically.
Hello!
Pull Request overview
kwargsinto parameters to allow IDE support when usinghuggingface.ChatCompletion.create.Optionalstatements.rtorequestto improve readability.Preface
First of all, this PR makes some style decisions, and I don't mean to impose my style onto this project, so feel free to suggest changes if they are not up to your likings.
Details
I've replaced
kwargsfromhuggingface.ChatCompletion.createwith the actual arguments used. This nearly matches theChatCompletionRequestmodel, with the exception of theuserfield (which I think might be unused?). Also,createhas adebugparameter that the model doesn't.I've also removed some
Optionalstatements. Note thatOptional[X]is equivalent toUnion[X, None], and for these parameters,Nonewas not a valid input. Some of the remaining ones, liketop_kandfrequency_penaltylikely also benefit from having theOptionalremoved, but they technically work withNone, so I left them for now. Feel free to let me know if you'd like these removed too.If desired, I can extrapolate these changes to the
CompletionandEmbeddingclasses, but I figured I'd let you have a look at this first.