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

langchain-tests: allow test_serdes for packages outside the default valid namespaces #30343

Merged
merged 1 commit into from
Mar 22, 2025

Conversation

mattf
Copy link
Contributor

@mattf mattf commented Mar 18, 2025

Description:

a third party package not listed in the default valid namespaces cannot pass test_serdes because the load() does not allow for extending the valid_namespaces.

test_serdes will fail with -
ValueError: Invalid namespace: {'lc': 1, 'type': 'constructor', 'id': ['langchain_other', 'chat_models', 'ChatOther'], 'kwargs': {'model_name': '...', 'api_key': '...'}, 'name': 'ChatOther'}

this change has test_serdes automatically extend valid_namespaces based off the ChatModel under test's namespace.

a third party package not listed in the default valid namespaces cannot
pass test_serdes because the load() does not allow for extending the
valid_namespaces.

test_serdes will fail with -
   ValueError: Invalid namespace: {'lc': 1, 'type': 'constructor', 'id': ['langchain_other', 'chat_models', 'ChatOther'], 'kwargs': {'model_name': '...', 'api_key': '...'}, 'name': 'ChatOther'}

this change has test_serdes automatically extend valid_namespaces
based off the ChatModel under test's namespace.
Copy link

vercel bot commented Mar 18, 2025

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 Mar 18, 2025 3:07pm

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. 🤖:nit Small modifications/deletions, fixes, deps or improvements to existing code or docs labels Mar 18, 2025
@mattf mattf changed the title standard-tests: allow test_serdes for packages outside the default valid namespaces langchain-tests: allow test_serdes for packages outside the default valid namespaces Mar 19, 2025
@mattf
Copy link
Contributor Author

mattf commented Mar 20, 2025

@ccurme ptal

@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Mar 22, 2025
@ccurme ccurme merged commit e703290 into langchain-ai:master Mar 22, 2025
124 checks passed
@mattf mattf deleted the allow-test-other-serdes branch March 23, 2025 17:14
mattf added a commit to mattf/langchain-llama-stack that referenced this pull request Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm PR looks good. Use to confirm that a PR is ready for merging. 🤖:nit Small modifications/deletions, fixes, deps or improvements to existing code or docs size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants