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

fix(runtimes): Remove unused Pydantic dependencies #1725

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

jesse-c
Copy link
Member

@jesse-c jesse-c commented Apr 29, 2024

These runtimes don't explicitly use Pydantic. Remove the dependency then to make that clear, and to increase flexibility. A step towards Pydantic v2.

@jesse-c jesse-c force-pushed the remove-unnecessary-pydantic-deps branch from 219dd31 to 5ed1fbb Compare April 29, 2024 15:08
@jesse-c jesse-c self-assigned this Apr 29, 2024
@jesse-c jesse-c marked this pull request as ready for review April 29, 2024 15:30
Copy link

@mauicv mauicv left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@sakoush sakoush left a comment

Choose a reason for hiding this comment

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

LGTM approving assuming comments are sorted before merging:

  • I have a comment on poetry version to be aligned with what we specify during docker build.

  • I also see no pyproject.toml changes for some runtimes e.g. alibi-explain so I am not sure if they were missed?

poetry.lock Show resolved Hide resolved
@jesse-c jesse-c force-pushed the remove-unnecessary-pydantic-deps branch from 5ed1fbb to 0226857 Compare April 30, 2024 10:25
@jesse-c
Copy link
Member Author

jesse-c commented Apr 30, 2024

I also see no pyproject.toml changes for some runtimes e.g. alibi-explain so I am not sure if they were missed?

This is for unused Pydantic dependency declarations. Alibi Explain uses Pydantic [1].

[1] https://github.com/jesse-c/MLServer/blob/02268574ff8cb5d330edfeb8cc10b64896472e22/runtimes/alibi-explain/mlserver_alibi_explain/common.py#L7

These runtimes don't explicitly use Pydantic. Remove the dependency
then to make that clear, and to increase flexibility.
@jesse-c jesse-c force-pushed the remove-unnecessary-pydantic-deps branch from 0226857 to 6bc0332 Compare April 30, 2024 10:48
@jesse-c jesse-c merged commit 3638be9 into SeldonIO:master Apr 30, 2024
20 checks passed
@jesse-c jesse-c deleted the remove-unnecessary-pydantic-deps branch April 30, 2024 11:04
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.

None yet

3 participants