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

Take decision on Pydantic version for core library and dev tools. #802

Closed
tcompa opened this issue Jul 18, 2024 · 2 comments · Fixed by #793
Closed

Take decision on Pydantic version for core library and dev tools. #802

tcompa opened this issue Jul 18, 2024 · 2 comments · Fixed by #793
Labels
dependencies Pull requests that update a dependency file

Comments

@tcompa
Copy link
Collaborator

tcompa commented Jul 18, 2024

(cc @lorenzocerrone @jluethi)

We still have something to clarify concerning the Pydantic dependency version.

  • After we fix Move core-library and tasks to Pydantic V2 #790 (e.g. as of Fully move to Pydantic v2 #793), fractal-tasks-core can only coexist with Pydantic v2. The most natural approach here would be to change the dependency constraint to pydantic>=2.
  • As part of Move JSON-schema generation to Pydantic v2 #791, we were planning to let the JSON-schema generation tools still work for Pydantic V1 as well. And in fact in Pydantic v2 second attempt jsonschema #801 I've been building towards a set of flexible schema-generation tools, which would generate either pydantic_v1 or pydantic_v2 manifests. Note that in this scenario we would need fractal-tasks-core to coexist with Pydantic v1 package, which is in contrast with the previous point.
  • A provisional workaround could go through import-time checks. However, since dev is nested within the core library main package (fractal_tasks_core) we don't have the option to be more flexible for the dev subpackage while being less flexible for its parent package. This is based on my understanding of import checks (see details below), but I'm curious to know about other options.

At the moments I don't see a way out. We would have to make the next fractal-tasks-core version depend on Pydantic V2. This means that v1.1.0 could be used as a transitional version, but package developers who import from fractal_tasks_core would not be able to use Pydantic V1 (this is probably a minor issue).
Two options would remain in terms of schema-generation tools:

  1. We keep them in sync with fractal_tasks_core, which means that they will only generate pydantic v2 schemas from now on.
  2. We move fractal_tasks_core.dev into a separate package, where we can keep its dependencies more flexible than the fractal_tasks_core ones.

I see the benefit of option 2, overall, but it's something that would clearly take more work. Option 1 is actually a no-effort change for any task developer who was not pinning their pydantic dependency, and probably would only affect very few users (who, for the moment, would need to pin fractal-tasks-core to be able to stick with Pydantic V1).


My understanding is based on the following example

$ tree .
.
└── fractal_tasks_core
    ├── core_library_module.py
    ├── dev
    │   ├── dev_module.py
    │   └── __init__.py
    └── __init__.py

2 directories, 4 files

$ cat fractal_tasks_core/__init__.py 
print("Printing from fractal_tasks_core/__init__.py")

$ cat fractal_tasks_core/dev/__init__.py 
print("Printing from fractal_tasks_core/dev/__init__.py")

$ python -c 'import fractal_tasks_core.dev.dev_module'
Printing from fractal_tasks_core/__init__.py
Printing from fractal_tasks_core/dev/__init__.py

where I am not able to bypass the import of the parent package when importing from dev.

I am happy to hear about something I missed.

@tcompa tcompa added the dependencies Pull requests that update a dependency file label Jul 18, 2024
@lorenzocerrone
Copy link
Collaborator

I would go for option 1 now. The number of packages with active development is limited, and helping them transition should not be too much work. Meanwhile, we can only pin version 1.1.0 for more stale packages for now.

That said, in the long run, I favor option 2. It would give us much more flexibility regarding back compatibility.
On pydantic website they say:

Pydantic V3 and beyond
We expect to make new major releases roughly once a year going forward, although as mentioned above, any associated breaking changes should be trivial to fix compared to the V1-to-V2 transition.

So, while changes might be smaller in the future, we can not exclude more inconsistencies in schema generation.

@jluethi
Copy link
Collaborator

jluethi commented Jul 18, 2024

We keep them in sync with fractal_tasks_core, which means that they will only generate pydantic v2 schemas from now on.

I see the benefit of option 2, overall, but it's something that would clearly take more work. Option 1 is actually a no-effort change for any task developer who was not pinning their pydantic dependency, and probably would only affect very few users (who, for the moment, would need to pin fractal-tasks-core to be able to stick with Pydantic V1).

Agreed. Let's go with option 1 now, I don't think that will be hard for current package developers to adopt. We can still think of having the dev package separate later :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
Development

Successfully merging a pull request may close this issue.

3 participants