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

update: llamaindex_conversable_agent.py #130

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

lazToum
Copy link
Collaborator

@lazToum lazToum commented Dec 2, 2024

Why are these changes needed?

If using pydantic > 2, we get AttributeError: type object 'Config' has no attribute 'copy'
A simple check is added to avoid this.

Related issue number

Checks

@marklysze
Copy link
Collaborator

Hey @lazToum! Thanks for putting these PRs through. Are you on Discord? If so can you connect to me, msze_, so we can send out a CLA for you?

@lazToum
Copy link
Collaborator Author

lazToum commented Dec 2, 2024

Hey @lazToum! Thanks for putting these PRs through. Are you on Discord? If so can you connect to me, msze_, so we can send out a CLA for you?

Hi @marklysze, sure, it's laztoum at discord. I have already signed one that Qingyun Wu sent me.

else:

class Config:
arbitrary_types_allowed = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @lazToum...

Can we provide a stricter comparison rather than directly with a string (e.g. 11.0 won't be greater 2.0):

from packaging import version
from pydantic import __version__ as pydantic_version

# This will correctly handle all future versions including v11+
if version.parse(pydantic_version) >= version.parse("2.0.0"):
...

Copy link
Collaborator

@marklysze marklysze Dec 3, 2024

Choose a reason for hiding this comment

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

Also, I tried to change my pydantic version to a lower version and it had an issue with llama index. I think it's because BeforeValidator isn't available in older versions of pydantic and the current llamaindex libraries use it.

For llamaindex do you think it would be better to force a minimum version of 2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this sounds good. Ideally, pydantic >=2 should be in the whole project, but I don't know if this is feasible (haven't checked all the extras).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants