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

tests[patch]: populate API reference for chat models #28487

Merged
merged 6 commits into from
Dec 3, 2024
Merged

Conversation

ccurme
Copy link
Collaborator

@ccurme ccurme commented Dec 3, 2024

Populate API reference for test class properties and test methods for chat models.

Also pytest.skip some tests that were previously passed if features are not supported.

@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Dec 3, 2024
Copy link

vercel bot commented Dec 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Dec 3, 2024 8:18pm

@@ -74,7 +74,7 @@ def _validate_tool_call_message_no_args(message: BaseMessage) -> None:

class ChatModelIntegrationTests(ChatModelTests):
@property
def standard_chat_model_params(self) -> dict:
def _standard_chat_model_params(self) -> dict:
Copy link
Member

Choose a reason for hiding this comment

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

is there a way to configure this via docstring, or does it have to be by name? just to avoid having to update all the implementations

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

From what I can tell the only integration that implements it is AWS. I added :meta private: to docstrings but it still seems to show up in the attributes table. Going with that for now.

I'm actually unclear on what is intended for this property. It is always combined with chat_model_params, so my guess was that we were testing a set of standard params (temperature, max_tokens, etc) that we want all models to support, and users should only overwrite chat_model_params. If that's correct, then we should update AWS to not override standard_chat_model_params.

Otherwise, we should document when/why this should be overwritten (as opposed to specifying things in chat_model_params).

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Dec 3, 2024
@ccurme ccurme merged commit ab831ce into master Dec 3, 2024
86 checks passed
@ccurme ccurme deleted the cc/update_tests branch December 3, 2024 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants