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

Pydantic API v2 #3034

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Pydantic API v2 #3034

wants to merge 3 commits into from

Conversation

loriab
Copy link
Member

@loriab loriab commented Aug 24, 2023

Description

Not expected to pass, as it needs a special qcel and qcng.

User API & Changelog headlines

  • RN 1
  • RN 2

Dev notes & details

  • Feature1
  • Feature2

Questions

  • Question1

Checklist

Status

  • Ready for review
  • Ready for merge

@loriab
Copy link
Member Author

loriab commented Nov 13, 2023

This could be tidied up and merged for v1.9, but that would constrict pydantic to only v2. From MolSSI/QCElemental#323, releases of qcfractal are only imminently even v2-tolerant. So I think it'd be safer to postpone and update the whole qca stack together. So not targeting v1.9. Any thoughts? @mattwthompson

@loriab loriab added this to the Psi4 1.10 milestone Nov 13, 2023
@mattwthompson
Copy link

My thoughts are

  • I'm jealous you were able to pull this off, I gave up in a fit of rage the last time I tried to do this migration
  • Having 1.9 drop with pydantic =2 constrained everywhere would be a small problem (I think that's what the contents of this PR would require?). We'd be stuck on 1.8.something, which isn't the end of the world but also not preferred.
  • Waiting until the QCArchive stack works with v1/v2 at installation time and then having the next-ish Psi4 release be pydantic =2 sounds like a great idea (I think this is what you're suggesting?)

@loriab
Copy link
Member Author

loriab commented Nov 14, 2023

  • I'm jealous you were able to pull this off, I gave up in a fit of rage the last time I tried to do this migration

Yeah, there were some bizarre bits -- had to basically restructure the driver import structure in #3033 to unwind the circular import to let pydantic see the classes it demanded. Thankfully Levi did the really hard part for qcel.

  • Having 1.9 drop with pydantic =2 constrained everywhere would be a small problem (I think that's what the contents of this PR would require?). We'd be stuck on 1.8.something, which isn't the end of the world but also not preferred.
  • Waiting until the QCArchive stack works with v1/v2 at installation time and then having the next-ish Psi4 release be pydantic =2 sounds like a great idea (I think this is what you're suggesting?)

Ok, good. Yes, I was suggesting psi v1.10 in May 2024 be pydantic=2 only and let v1.9 stay v1/v2-compatible. I don't want downstream to get locked into too-narrow a slice of the stack to cope with other constraints. So long as psi4 as v1/v2 isn't holding openff back, I think this is the way to go. Thanks!

@mattwthompson
Copy link

That timeline will work for us - hopefully we're properly on v2 but at very least I'd expect us to be in the v1/v2 compatible state. (I hope this doesn't take much more than a QCFractal release and us putting the import guards in a couple of our packages.) Thanks for the heads-up!

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.

None yet

2 participants