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

core: fixed circular dependency with json schema #18657

Merged
merged 6 commits into from Mar 12, 2024

Conversation

thebhulawat
Copy link
Contributor

@thebhulawat thebhulawat commented Mar 6, 2024

Description: Circular dependencies when parsing references leading to RecursionError: maximum recursion depth exceeded issue. This PR address the issue by handling previously seen refs as in any typical DFS to avoid infinite depths.

Issue: #12163

Twitter handle: https://twitter.com/theBhulawat

  • Add tests and docs: If you're adding a new integration, please include

    1. a test for the integration, preferably unit tests that do not rely on network access,
    2. an example notebook showing its use. It lives in docs/docs/integrations directory.
  • Lint and test: Run make format, make lint and make test from the root of the package(s) you've modified. See contribution guidelines for more: https://python.langchain.com/docs/contributing/

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. 🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature labels Mar 6, 2024
Copy link

vercel bot commented Mar 6, 2024

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Mar 12, 2024 5:40am

@eyurtsev
Copy link
Collaborator

eyurtsev commented Mar 6, 2024

Awesome @namsgit would you be wiliing to add a unit test and type hints?

@baskaryan baskaryan added the needs test PR needs to be updated with tests label Mar 8, 2024
@thebhulawat
Copy link
Contributor Author

@eyurtsev Thank you. I have added tests.

@thebhulawat
Copy link
Contributor Author

@eyurtsev Requesting for your review.

@thebhulawat
Copy link
Contributor Author

@baskaryan Sorry if I am bothering but if you or eyurtsev can have a look at the PR, would be great!

@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Mar 12, 2024
@baskaryan baskaryan enabled auto-merge (squash) March 12, 2024 05:40
@baskaryan baskaryan merged commit 7512264 into langchain-ai:master Mar 12, 2024
95 checks passed
bechbd pushed a commit to bechbd/langchain that referenced this pull request Mar 29, 2024
…#18657)

**Description:** Circular dependencies when parsing references leading
to `RecursionError: maximum recursion depth exceeded` issue. This PR
address the issue by handling previously seen refs as in any typical DFS
to avoid infinite depths.

**Issue:** langchain-ai#12163

 **Twitter handle:** https://twitter.com/theBhulawat 


- [x] **Add tests and docs**: If you're adding a new integration, please
include
1. a test for the integration, preferably unit tests that do not rely on
network access,
2. an example notebook showing its use. It lives in
`docs/docs/integrations` directory.


- [x] **Lint and test**: Run `make format`, `make lint` and `make test`
from the root of the package(s) you've modified. See contribution
guidelines for more: https://python.langchain.com/docs/contributing/

---------

Co-authored-by: Bagatur <baskaryan@gmail.com>
Co-authored-by: Bagatur <22008038+baskaryan@users.noreply.github.com>
gkorland pushed a commit to FalkorDB/langchain that referenced this pull request Mar 30, 2024
…#18657)

**Description:** Circular dependencies when parsing references leading
to `RecursionError: maximum recursion depth exceeded` issue. This PR
address the issue by handling previously seen refs as in any typical DFS
to avoid infinite depths.

**Issue:** langchain-ai#12163

 **Twitter handle:** https://twitter.com/theBhulawat 


- [x] **Add tests and docs**: If you're adding a new integration, please
include
1. a test for the integration, preferably unit tests that do not rely on
network access,
2. an example notebook showing its use. It lives in
`docs/docs/integrations` directory.


- [x] **Lint and test**: Run `make format`, `make lint` and `make test`
from the root of the package(s) you've modified. See contribution
guidelines for more: https://python.langchain.com/docs/contributing/

---------

Co-authored-by: Bagatur <baskaryan@gmail.com>
Co-authored-by: Bagatur <22008038+baskaryan@users.noreply.github.com>
hinthornw pushed a commit that referenced this pull request Apr 26, 2024
**Description:** Circular dependencies when parsing references leading
to `RecursionError: maximum recursion depth exceeded` issue. This PR
address the issue by handling previously seen refs as in any typical DFS
to avoid infinite depths.

**Issue:** #12163

 **Twitter handle:** https://twitter.com/theBhulawat 


- [x] **Add tests and docs**: If you're adding a new integration, please
include
1. a test for the integration, preferably unit tests that do not rely on
network access,
2. an example notebook showing its use. It lives in
`docs/docs/integrations` directory.


- [x] **Lint and test**: Run `make format`, `make lint` and `make test`
from the root of the package(s) you've modified. See contribution
guidelines for more: https://python.langchain.com/docs/contributing/

---------

Co-authored-by: Bagatur <baskaryan@gmail.com>
Co-authored-by: Bagatur <22008038+baskaryan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature lgtm PR looks good. Use to confirm that a PR is ready for merging. needs test PR needs to be updated with tests size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants