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.

Sign up for GitHub

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

Jump to bottom

partners: replace deprecated .dict() with .model_dump() in openai base.py #16629

Closed
wants to merge 4 commits into from
Closed

partners: replace deprecated .dict() with .model_dump() in openai base.py #16629

wants to merge 4 commits into from

Conversation

SavvasMohito
Copy link
Contributor

Thanks :)

@SavvasMohito
replace deprecated .dict() with .model_dump()
8718319 
using pydantic's new function. it produces the same result without throwing a warning console message
@efriis efriis added the partner label Jan 26, 2024
@efriis efriis self-assigned this Jan 26, 2024
@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Jan 26, 2024
@vercel Vercel
Copy link

vercel bot commented Jan 26, 2024 •
edited

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 Jan 29, 2024 9:48pm

@dosubot dosubot bot added Ɑ: models Related to LLMs or chat model modules 🤖:improvement Medium size change to existing code to handle new use-cases labels Jan 26, 2024
@eyurtsev
Copy link
Collaborator

LangChain needs to be both pydantic 1 and 2 compatible for now.

Running tests expecting the tests to fail. If they don't fail we should confirm why not.

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Jan 29, 2024
@SavvasMohito
Copy link
Contributor Author

@eyurtsev The tests do fail as you suspected. I made an additional commit to satisfy both v1 and v2 (and 3) but also avoid the deprecation warnings. Local tests do not throw any errors on my end

@hwchase17 hwchase17 closed this Jan 30, 2024
@baskaryan baskaryan reopened this Jan 30, 2024
@SavvasMohito
Copy link
Contributor Author

so no merge @baskaryan? 😔

@baskaryan baskaryan assigned eyurtsev and unassigned efriis Feb 5, 2024
@sam-cohan
Copy link

When do we expect this to be solved, it is peppering our code with lots of warnings...

@sam-cohan
Copy link

btw, Ideally the PR should cover all instances of this problem (for example the embeddings module has the same issue)

@baskaryan
Copy link
Collaborator

@eyurtsev i actually think any OpenAI-returned object, whether you've got pydantic v1 or v2 installed, should support model_dump https://github.com/openai/openai-python/blob/d231d1fa783967c1d3a1db3ba1b52647fff148ac/src/openai/_models.py#L150

@SavvasMohito
Copy link
Contributor Author

great observation @baskaryan. they seem to be using .dict() under the hood here but through the model_dump() function name in case of pydantic v1.

@eyurtsev
Copy link
Collaborator

eyurtsev commented Feb 9, 2024 •
edited

Ah if OpenAI already supports it we can just upgrade. Shall we just change to model_dump version?

@SavvasMohito SavvasMohito closed this by deleting the head repository Feb 12, 2024
@SavvasMohito SavvasMohito mentioned this pull request Feb 12, 2024
Merged
@SavvasMohito
Copy link
Contributor Author

Closed by mistake, re-opened in #17404. Sorry 😢

efriis pushed a commit that referenced this pull request Feb 21, 2024
@SavvasMohito @baskaryan
partners/openai: fix depracation errors of pydantic's .dict() functio…
691ff67 
…n (reopen #16629) (#17404)



---------

Co-authored-by: Bagatur <baskaryan@gmail.com>
k8si pushed a commit to Mozilla-Ocho/langchain that referenced this pull request Feb 22, 2024
@SavvasMohito @baskaryan @k8si
partners/openai: fix depracation errors of pydantic's .dict() functio…
541f557 
…n (reopen langchain-ai#16629) (langchain-ai#17404)



---------

Co-authored-by: Bagatur <baskaryan@gmail.com>
al1p pushed a commit to al1p/langchain that referenced this pull request Feb 27, 2024
@SavvasMohito @baskaryan @al1p-R
partners/openai: fix depracation errors of pydantic's .dict() functio…
be60635 
…n (reopen langchain-ai#16629) (langchain-ai#17404)



---------

Co-authored-by: Bagatur <baskaryan@gmail.com>
haydeniw pushed a commit to haydeniw/langchain that referenced this pull request Feb 27, 2024
@SavvasMohito @baskaryan
partners/openai: fix depracation errors of pydantic's .dict() functio…
e6b8a8b 
…n (reopen langchain-ai#16629) (langchain-ai#17404)



---------

Co-authored-by: Bagatur <baskaryan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@eyurtsev eyurtsev

Labels
🤖:improvement Medium size change to existing code to handle new use-cases Ɑ: models Related to LLMs or chat model modules partner size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@SavvasMohito @eyurtsev @sam-cohan @baskaryan @efriis @hwchase17