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

Use Pydantic v1 API even with pydantic v2 #314

Merged
merged 5 commits into from
Jul 31, 2023

Conversation

Lnaden
Copy link
Collaborator

@Lnaden Lnaden commented Jul 28, 2023

Description

This PR changes all imports to test against Pydantic v2 and then uses the v1 api if found. Precursor to future v2 migration.

Also updates the CI to test against both versions. Hopefully I figured out Poetry correctly.

Changelog description

Compatibility with Pydantic v2 but use the v1 API as a stopgap for the version pin.

Status

  • Code base linted
  • Ready to go

This PR changes all imports to test against Pydantic v2 and then uses the v1 api if found. Precursor to future v2 migration.

Also updates the CI to test against both versions. Hopefully I figured out Poetry correctly.
@loriab
Copy link
Collaborator

loriab commented Jul 29, 2023

I like your import pydantic.v1 plan to allow more pydantic versions.

From https://stackoverflow.com/a/70632360 , I think your attempts to manipulate poetry in the GHA through pyproject.toml groups might not be allowed b/c extras not exclusive. Poetry seems ... uncooperative ... for this purpose.

I wonder if a workaround is to return pyproject.toml to pydantic >=1.8.2 (approx.). Then for the GHA with pydantic=1 in build matrix, do a sed on pyproject.toml to turn it to pydantic >=^1.8.2. Might work?

@Lnaden
Copy link
Collaborator Author

Lnaden commented Jul 29, 2023

The import pydantic.v1 was something they added to the v2 of pydantic in case you wanted the API from v1 still, so its not long term stable, but stable enough.

I was worried about the Poetry part because it does say it'll resolve all optional dependencies in the tree too, which really makes me wonder why they don't allow superseding versions of dependencies such as for dev versions.

Then for the GHA with pydantic=1 in build matrix, do a sed on pyproject.toml to turn it to pydantic >=^1.8.2. Might work?

I'm willing to try something to this effect, I just wont have time to get to it this weekend, so don't let this PR get in the way of other PR's/releases you want to do.

…inplace sed to pin pydantic version to 1.0 (assuming the spec remains >=1.y.z for now)
…o everything just ever so slightly differently.
@codecov
Copy link

codecov bot commented Jul 31, 2023

Codecov Report

Merging #314 (398be4c) into master (7b04890) will decrease coverage by 0.12%.
The diff coverage is 79.48%.

Additional details and impacted files

@Lnaden
Copy link
Collaborator Author

Lnaden commented Jul 31, 2023

Okay, this should be ready to go! Failing error is not related to the functional code (just the doc building which is also pydantic problems but not in the way of using the code)

@loriab the sed idea worked great, it does a replacement for checking against v1. I left my 'what the hell does this sed string do' comments in place in cased someone else has to edit it that isn't me.

Copy link
Collaborator

@loriab loriab left a comment

Choose a reason for hiding this comment

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

Hooray, thanks for finding that pydantic.v1 workaround so we don't have to be so restrictive. I'll give the build error a try elsewhere.

@Lnaden
Copy link
Collaborator Author

Lnaden commented Jul 31, 2023

The build error has to do with the autodoc-pydantic program still using BaseSettings from v1 pydantic but on loading up v2 it cant find it because that class moved to a different module. So, at least as pinned, this is an upstream problem but only for doc building, not for functionality.

@loriab
Copy link
Collaborator

loriab commented Jul 31, 2023

The build error has to do with the autodoc-pydantic program still using BaseSettings from v1 pydantic but on loading up v2 it cant find it because that class moved to a different module. So, at least as pinned, this is an upstream problem but only for doc building, not for functionality.

Ok, I was thinking your sed line could be copied into the build_documentation part of the GHA, so it'd generate docs with v1 until upstream gets straightened out. Just lmk if you want to do that here or if I should merge it and try it at #311

@Lnaden
Copy link
Collaborator Author

Lnaden commented Jul 31, 2023

Thats actually a great idea. I've added the sed here for the build_documentation directive, I think that will resolve all the issues and we can re-touch on #311 after.

@loriab loriab merged commit f7e404c into MolSSI:master Jul 31, 2023
8 checks passed
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.

2 participants