Skip to content

Bump pydantic version #577

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

Closed
wants to merge 1 commit into from
Closed

Bump pydantic version #577

wants to merge 1 commit into from

Conversation

khiemdoan
Copy link

@khiemdoan khiemdoan commented Nov 1, 2023

Solved #546

@edwardzjl
Copy link

Hello @chayim I don't know who to pin so I random picked you, and I apologize if this choice is inappropriate.

As there has been several PRs trying to upgrade pydantic, could you kindly review one and provide some progress updates?

Additionally, I'm interested in the current status of redis-om since it hasn't seen updates for several months. Is the project in maintenance mode?

@@ -36,7 +36,7 @@ include=[
[tool.poetry.dependencies]
python = ">=3.8,<4.0"
redis = ">=3.5.3,<5.0.0"
pydantic = ">=1.10.2,<2.1.0"
pydantic = ">=1.10.2,<3.0.0"

Choose a reason for hiding this comment

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

I'd remove the upper bound entirely, see this article: https://arc.net/l/quote/txkfghki

Copy link
Author

Choose a reason for hiding this comment

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

I don't agree with that.
Pydantic v3 would have breaking changes, see: Pydantic V3 and beyond
Nothing promises it will be compatible with older versions. Especially this library is still both Pydantic v1 and v2.

Choose a reason for hiding this comment

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

yes, that'd be the cause, but there might be a chance where this library wouldn't be affected by the breaking changes :)
And if you're affected by the breaking changes you can pin your side 😊

@khiemdoan
Copy link
Author

Solve #546 #548

@hthall13 hthall13 mentioned this pull request Feb 2, 2024
@gaby
Copy link
Contributor

gaby commented Mar 7, 2024

@khiemdoan This actually fails when running make lint

@edwardzjl
Copy link

@gaby I believe it's the black version (from 23.x to 24.x) that breaks the lint.

Upgrading pydantic version has nothing to do with lint.

@gaby
Copy link
Contributor

gaby commented Mar 7, 2024

@edwardzjl The mypy checks fail. Black is working fine. I have a fork where i'm merging all the PR's

The highest I can bump pydantic is to <2.5.0, anything higher than that has a ton of failures. I also tried enabling the pydantic-mypy plugin and there's +300 errors.

@gaby
Copy link
Contributor

gaby commented Mar 7, 2024

@edwardzjl Using anything higher than pydantic<2.5.0 throws this error:

ubuntu@ubuntu:~/Desktop/git/fix/redis-om-python$ make lint
/usr/local/bin/poetry install
Updating dependencies
Resolving dependencies... (1.9s)

Package operations: 0 installs, 2 updates, 0 removals

  - Updating pydantic-core (2.10.1 -> 2.14.6)
  - Updating pydantic (2.4.2 -> 2.5.3)

Writing lock file

Installing the current project: redis-om (0.2.1)
touch .install.stamp
find . -type d -name "__pycache__" | xargs rm -rf {};
rm -rf .install.stamp .coverage .mypy_cache
rm -rf build
rm -rf dist
rm -rf redis_om
rm -rf tests_sync
docker compose down
/usr/local/bin/poetry run python make_sync.py
/usr/local/bin/poetry build
Building redis-om (0.2.1)
  - Building sdist
  - Built redis_om-0.2.1.tar.gz
  - Building wheel
  - Built redis_om-0.2.1-py3-none-any.whl
/usr/local/bin/poetry run isort --profile=black --lines-after-imports=2 ./tests/ aredis_om redis_om
/usr/local/bin/poetry run black ./tests/ aredis_om
All done! ✨ 🍰 ✨
25 files left unchanged.
/usr/local/bin/poetry run flake8 --ignore=W503,E501,F401,E731 ./tests/ aredis_om redis_om
/usr/local/bin/poetry run mypy ./tests/ aredis_om redis_om --ignore-missing-imports --exclude migrate.py --exclude _compat\.py$
Traceback (most recent call last):
  File "/home/ubuntu/.cache/pypoetry/virtualenvs/redis-om--qx16He5-py3.10/bin/mypy", line 8, in <module>
    sys.exit(console_entry())
  File "/home/ubuntu/.cache/pypoetry/virtualenvs/redis-om--qx16He5-py3.10/lib/python3.10/site-packages/mypy/__main__.py", line 15, in console_entry
    main()
  File "mypy/main.py", line 95, in main
  File "mypy/main.py", line 174, in run_build
  File "mypy/build.py", line 187, in build
  File "mypy/build.py", line 270, in _build
  File "mypy/build.py", line 2867, in dispatch
  File "mypy/build.py", line 3251, in process_graph
  File "mypy/build.py", line 3372, in process_stale_scc
  File "mypy/build.py", line 2442, in write_cache
  File "mypy/build.py", line 1559, in write_cache
  File "mypy/nodes.py", line 386, in serialize
  File "mypy/nodes.py", line 3649, in serialize
  File "mypy/nodes.py", line 3581, in serialize
AssertionError: Definition of pydantic.types.JsonValue is unexpectedly incomplete
make: *** [Makefile:57: lint] Error 1

@gaby
Copy link
Contributor

gaby commented Mar 7, 2024

Just tested using pydantic<3.0.0 and mypy has the same error.

@edwardzjl
Copy link

@gaby I'm sorry I confused poetry version syntax with pipenv, the version of black is limited to 23.x. I have bad memories about upgrading black to 24.x. Also I did not realize that mypy will scan dependency files.

After some tests I confirm that using pydantic < 2.5.0 works.

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.

4 participants