Consider dropping Pydantic

[Secrus]

Will you consider dropping pydantic in favor of dataclasses or just plain Python objects? pydantic recently released v2 which has its core rewritten in Rust. That will cause major issues when trying to vendor pythonfinder in projects like pipenv.

[dvzrv]

FWIW, making it work with >= 2.0 is trivial: https://gitlab.archlinux.org/archlinux/packaging/packages/python-pythonfinder/-/blob/main/python-pythonfinder-2.0.4-pydantic2.patch

I'm currently doing rebuilds on Arch Linux for pydantic >= 2.0

[Secrus]

FWIW, making it work with >= 2.0 is trivial: https://gitlab.archlinux.org/archlinux/packaging/packages/python-pythonfinder/-/blob/main/python-pythonfinder-2.0.4-pydantic2.patch

I'm currently doing rebuilds on Arch Linux for pydantic >= 2.0

Yes, but in the case of vendoring dependencies and the Python distribution model, it works a bit differently. Linux packaging is different from Python packaging.

[matteius]

I am not in favor of removing pydantic, I've put quite a bit of time into replacing attr/attrs in pythonfinder, requirementslib and pipenv. We are trying to further overhaul requirementslib and want to make use of the validations with plans to backport changes that are made directly in vendor'd requirementslib back to requirementslib the package on some cadence or interval. We have no immediate plans to upgrade past pydantic 1.x and may revisit how the vendoring is done in pipenv before we would consider 2.x. Either way, changing vendoring or not, it will require serious considerations to go to pydantic 2 and there are more fundamental bugs in requirementslib that should be addressed first.

[Secrus]

I understand that, but still, I believe that pydantic should be dropped. Their migration to Rust want be taken back and one day they won't support v1 anymore. For now, at the very least, you could consider restricting pydantic version to <2 to make sure that it will continue working.

[matteius]

Is your objection to just pythonfinder using it because something other than pipenv is using pythonfinder or more broadly speaking? I think to get to pydantic 2, which has some benefits of being faster, we would possible need to not vendor pydantic but install it alongside pipenv. I've spent a bit of time looking more at how poetry installs itself for possible inspiration.

[Secrus]

well, I am part of Poetry core team and we are looking into getting pythonfinder or a similar library to use in our code for finding proper python executables. Recently we had an issue with jsonschema suddenly depending on a Rust-based library, which made me think about how many of our dependencies are using native extensions and whether we need all of them. Personally I am not against pydantic, but I see it as kinda more problematic than beneficial when it comes to libraries. It's fine when we are considering applications (like web apps built with pydantic and fastapi)

I've spent a bit of time looking more at how poetry installs itself for possible inspiration.

We are migrating to suggesting pipx as the default installation method, with our script as "second best".

[matteius]

@Secrus That is interesting, I would be open to a PR that converts pythonfinder to vanilla dataclasses; It would be cool to see poetry using pythonfinder if you head in that direction. I think as far as requirementslib goes, it will keep pydantic for the foreseeable future, at least until that library can be cleaned up but that likely wouldn't affect you.

Just a note on latest pythonfinder areas of improvement:

• when I dropped pep514tools to standardize the codepaths I didn't realize it was providing some kind of py command support for windows. I think it should be supported again but natively to pythonfinder without re-adding pep514tools`` Pythonfinder should support the windows py command pypa/pipenv#5779
• pythonfinder is a tad bit slower in Windows which is evidenced by the pipenv CI in the setup step -- for whatever reason the virtualenv creation and possibly finding python takes just marginally longer on windows -- I haven't quite figured out why or if it can be improved, it seems odd since the test CI is passing in the path to python so it shouldn't really be "finding" anything differently there.

We are migrating to suggesting pipx as the default installation method, with our script as "second best".

That is also interesting, admittedly I know how much about pipx but I wouldn't want to require another package manager that isn't the standard python package manager pip to install pipenv for example. What is the rationale for heading towards pipx?

[dvzrv]

Restricting this package to v1 only is problematic for downstream distributions, which now have to carry patches and continuously rework them (depending on upstream activity).

Could this not have been turned into a conditional import? 😢

[matteius]

Restricting this package to v1 only is problematic for downstream distributions

@dvzrv I am not sure what you mean seeing as pythonfinder won't run on pydantic 2 without code changes.

Aside -- I am in favor of removing pydantic to vanilla dataclasses, but I don't have the time to take it on myself at present.

[dvzrv]

pythonfinder won't run on pydantic 2 without code changes.

Yeah, the changes are fairly trivial though (see https://gitlab.archlinux.org/archlinux/packaging/packages/python-pythonfinder/-/blob/main/python-pythonfinder-2.0.5-pydantic2.patch) and could be turned into something that works across 1 and 2 for the time being.

I am in favor of removing pydantic to vanilla dataclasses, but I don't have the time to take it on myself at present.

Makes sense. I'll wait then :)

[oz123]

Late to join the party, but still.
I would also like to see a pydantic removed in favor of pure dataclasses. BUT, who is going to do this? It is a huge amount of work to do... If you are willing to put the effort, I think we should merge such a PR.

[Secrus]

@oz123 I am up to make the effort, just been busy with other projects lately.

[oz123]

I'd love to collaborate on that. I am also trying to do the same on plette.

[dvzrv]

It would have been great not having to backport the aforementioned pydantic >= 2 patches when packaging for downstream distributions for the latest release (my patch no longer applies, and this extra work has to be done by all downstreams working with pydantic 2).

Can we please patch this (I don't really care if the version constraint remains, as we remove that anyways) for the time being? :)

[dvzrv]

For anyone needing this, here is a patch for making pythonfinder work with pydantic v2:

diff --git i/pyproject.toml w/pyproject.toml
index 73a3c85..6cf3c2e 100644
--- i/pyproject.toml
+++ w/pyproject.toml
@@ -36,7 +36,7 @@ requires-python = ">=3.7"
 dependencies = [
     "cached-property; python_version < \"3.8\"",
     "packaging>=22.0",
-    "pydantic>=1.10.7,<2",
+    "pydantic>2",
 ]
 
 [project.optional-dependencies]
diff --git i/src/pythonfinder/models/common.py w/src/pythonfinder/models/common.py
index 4c439c9..0ef3d77 100644
--- i/src/pythonfinder/models/common.py
+++ w/src/pythonfinder/models/common.py
@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from pydantic import BaseModel, Extra
+from pydantic.v1 import BaseModel, Extra
 
 
 class FinderBaseModel(BaseModel):
diff --git i/src/pythonfinder/models/mixins.py w/src/pythonfinder/models/mixins.py
index 58ce99a..e68020f 100644
--- i/src/pythonfinder/models/mixins.py
+++ w/src/pythonfinder/models/mixins.py
@@ -12,7 +12,7 @@ from typing import (
     Optional,
 )
 
-from pydantic import BaseModel, Field, validator
+from pydantic.v1 import BaseModel, Field, validator
 
 from ..exceptions import InvalidPythonVersion
 from ..utils import (
diff --git i/src/pythonfinder/models/path.py w/src/pythonfinder/models/path.py
index fe98054..beb88be 100644
--- i/src/pythonfinder/models/path.py
+++ w/src/pythonfinder/models/path.py
@@ -23,7 +23,7 @@ if sys.version_info >= (3, 8):
     from functools import cached_property
 else:
     from cached_property import cached_property
-from pydantic import Field, root_validator
+from pydantic.v1 import Field, root_validator
 
 from ..environment import (
     ASDF_DATA_DIR,
diff --git i/src/pythonfinder/models/python.py w/src/pythonfinder/models/python.py
index c5e0345..32c82a8 100644
--- i/src/pythonfinder/models/python.py
+++ w/src/pythonfinder/models/python.py
@@ -19,7 +19,7 @@ from typing import (
 )
 
 from packaging.version import Version
-from pydantic import Field, validator
+from pydantic.v1 import Field, validator
 
 from ..environment import ASDF_DATA_DIR, PYENV_ROOT, SYSTEM_ARCH
 from ..exceptions import InvalidPythonVersion

[oz123]

Why not submit a proper PR?

[yurivict]

The current requirement of pydantic<2 here conflicts with PYPI having pydantic at 2.5.3.

The above patch should be either committed, or Pydantic should be dropped.

[matteius]

The above patch should be either committed, or Pydantic should be dropped.

We need to do both ... would have been helpful to open that patch as a PR, but I can probably manage it; just have been real busy.

It seems just copying that patch doesn't apply directly:

$ git apply patch.diff
error: patch failed: pyproject.toml:36
error: pyproject.toml: patch does not apply
error: patch failed: src/pythonfinder/models/path.py:23
error: src/pythonfinder/models/path.py: patch does not apply

[matteius]

Well the tests on dropping pydantic work, opening a PR against pipenv to see how those tests do with it. #157

[matteius]

OK, ready for review -- we have a clean test run here and in pipenv with these changes: #157