-
Notifications
You must be signed in to change notification settings - Fork 163
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
vertexai[patch]: support model
param
#81
Conversation
bf56160
to
8b6122f
Compare
8b6122f
to
5ddc4f0
Compare
@@ -55,6 +55,12 @@ class _VertexAIBase(BaseModel): | |||
model_name: Optional[str] = None | |||
"Underlying model name." | |||
|
|||
@root_validator(pre=True) |
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.
should we add an @property
for model as well?
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.
do you know any use case when it's actually consumed by any chaiins or other "external" instances?
if model
property is commonly used outside the LLM/ChatModel itself, maybe we should actually deprecate model_name
then.
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.
do you know any use case when it's actually consumed by any chaiins or other "external" instances? if
model
property is commonly used outside the LLM/ChatModel itself, maybe we should actually deprecatemodel_name
then.
I think we should use model_name
to stay consistent with other provider.
See this issue
#81
@lkuligin @efriis
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.
I'll merge this one as is then, and we can add another property later if we decide so.
Related to langchain-ai/langchain#18975
Didn't go the usual pydantic
Field(..., alias="model")
route because that would've caused mypy errors for anyone passing inmodel_name
(ie for all existing code).