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

Request: bumping inference extra to pydantic v2 #1726

Closed
jamesbraza opened this issue Oct 12, 2023 · 6 comments · Fixed by #1727
Closed

Request: bumping inference extra to pydantic v2 #1726

jamesbraza opened this issue Oct 12, 2023 · 6 comments · Fixed by #1727

Comments

@jamesbraza
Copy link
Contributor

Is your feature request related to a problem? Please describe.

The inference extra requires pydantic v1 for text-generation-inference: https://github.com/huggingface/huggingface_hub/blob/v0.18.0/setup.py#L33

However, thanks to huggingface/text-generation-inference#900, released in https://github.com/huggingface/text-generation-inference/releases/tag/v1.1.0, pydantic v2 is now supported there.

So this means inference extra can now support pydantic v2 as well! 🥳

Describe the solution you'd like

Allowing pydantic v2 with Hugging Face's Inference API client.

Describe alternatives you've considered

n/a

Additional context

n/a

@Wauplin
Copy link
Contributor

Wauplin commented Oct 12, 2023

Hi @jamesbraza thanks for the suggestion! In general, I'd be in favor of relaxing the dependency version :)

Just to be sure, what does that imply in term of backward-compatibility? (I would prefer the InferenceClient to stay compatible with an earlier version of TGI as far as possible).

EDIT: oh and thanks for the PR as well! ❤️

@jamesbraza
Copy link
Contributor Author

Yeah, thanks! 👍

The pydantic upgrade in TGI was backwards compatible, so this PR will then also be equally compatible. In other words, from what I am seeing, it won't affect compatibility because it's just a range loosening

@jamesbraza
Copy link
Contributor Author

The source code was compatible, but actually the test suite wasn't. I thus opened two PRs to make the test suite compatible:

@Wauplin
Copy link
Contributor

Wauplin commented Oct 13, 2023

Thanks for opening PRs fixing these @jamesbraza!

It's too bad that I don't remember why I've pinned pydantic version in the first place 😕 (I did a commit on main without description and bad practices always hits you back 😭). Maybe it was because of the webhook payload test you fixed but I'm not sure. I'll check and make a few tests with pydantic 1.0 vs 2.0 just to be sure it works as expected.

@Wauplin
Copy link
Contributor

Wauplin commented Oct 13, 2023

Merged! :)

@jamesbraza
Copy link
Contributor Author

Happy to help 👍

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 a pull request may close this issue.

2 participants