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: bump packaging dep to v24 #21142

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dokterbob
Copy link

Allow installing core with packaging@^24.

Copy link

vercel bot commented May 1, 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 May 18, 2024 3:07pm

@dokterbob dokterbob marked this pull request as ready for review May 1, 2024 09:48
@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label May 1, 2024
@@ -15,7 +15,7 @@ langsmith = "^0.1.0"
tenacity = "^8.1.0"
jsonpatch = "^1.33"
PyYAML = ">=5.3"
packaging = "^23.2"
packaging = ">=23.2,^24.0"
Copy link
Collaborator

@eyurtsev eyurtsev May 1, 2024

Choose a reason for hiding this comment

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

>=23.2 implies 24 and 25 if i'm not mistaken?

Copy link
Author

Choose a reason for hiding this comment

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

>= equal or higher than.

AFAIK this implies higher than or equal to 23.2 and lower than 25.

Ref: https://python-poetry.org/docs/dependency-specification/

Copy link
Collaborator

Choose a reason for hiding this comment

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

afaik the , indicates a logical AND not a logical OR, so >=23.2,^24.0 ends up just being ^24.0. should it isntead be >=23.2,<25?

@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label May 13, 2024
@eyurtsev eyurtsev enabled auto-merge (squash) May 13, 2024 15:17
@eyurtsev
Copy link
Collaborator

Thanks! We'll try to see if we can remove packaging as a dependency to avoid dependency conflicts for users

auto-merge was automatically disabled May 18, 2024 15:06

Head branch was pushed to by a user without write access

@dokterbob
Copy link
Author

Thanks! We'll try to see if we can remove packaging as a dependency to avoid dependency conflicts for users

That's great to hear but in the mean time, perhaps we could use my code to fix the issue for the time being?

Just rebased my PR to master.

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. size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants