-
-
Notifications
You must be signed in to change notification settings - Fork 334
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
Validate Pydantic imports #546
Conversation
f0eeb7f
to
252f167
Compare
Weird, I'm getting an unrelated error that I've never seen before. Seems like the build environment is missing the latest Full logs: https://github.com/jupyterlab/jupyter-ai/actions/runs/7333155619/job/19968249161?pr=546 Excerpt:
|
I found a related issue: python-attrs/attrs#1210 For some reason, |
2cb01ea
to
b0e0258
Compare
I had to wrap everything in a virtual environment, and now the job fails correctly, reporting the import added in #398 on Full logs: https://github.com/jupyterlab/jupyter-ai/actions/runs/7333298903/job/19968577119?pr=546 |
b0e0258
to
cf9e051
Compare
Pushed the commit to fix the pydantic import. I also had to follow this, since I renamed the other lint job: https://github.com/orgs/community/discussions/26698 |
4c6dad9
to
e2c8976
Compare
Happy to see more linting for conventions 🎉 On some loosely related things which may or may not deserve their own tickets:
|
Yup, these are all great suggestions! We're doing the best we can with the resources we have to spare. Feel free to open issues for any suggestions you have. For discussion on more minor topics that may not warrant their own issue, you can email me directly. I can then establish some kind of communication channel between you and our team. This seems like a good idea given how much you've been contributing lately. |
* lint * add import linter job in GH Actions * fix pydantic import
* lint * add import linter job in GH Actions * fix pydantic import
Description
Adds a GH Actions job that fails if any of our packages import from
pydantic
directly.Context
(cc @JasonWeill)
While reviewing @krassowski 's work, I noticed that both #398 and #465 were importing from
pydantic
directly. To be compatible with Pydantic v1 and v2, we need to pick an API to use and make sure the way we are using it works regardless of whether the user's environment contains Pydantic v1 or v2. Currently, we do so by importing fromlangchain.pydantic_v1
, which exposes the v1 API in both cases. We do not want to import frompydantic
directly as that will resolve to Pydantic v1 or v2, depending on which is installed in the user's environment.