search

MolSSI / QCFractal

A distributed compute and database platform for quantum chemistry.
https://molssi.github.io/QCFractal/
BSD 3-Clause "New" or "Revised" License
144 stars 47 forks source link

Enable Pydantic v2 using v1 API #736

Closed Lnaden closed 9 months ago

Lnaden commented 1 year ago

Description

As it says. This is a stopgap until we can properly implement Pydantic v2

Focuses mostly on QCPortal

Part of the larger QCA Upgrade to Pydantic v2

Changelog description

Status

loriab commented 1 year ago

ok, qcng 0.27 is up, so I think you're good to iterate on this. note that I pinned black to 22 over at qcng, so possibly that and similar maintenance changes there could be useful here. qcel black is pinned at 22 but I'll backtrack that sometime for consistency.

codecov[bot] commented 1 year ago

Codecov Report

Merging #736 (777d181) into next (d5aa7bb) will decrease coverage by 0.26%. The diff coverage is 43.81%.

Additional details and impacted files
loriab commented 1 year ago

I'm pretty confident that all the real tests will pass now here. Docs build after some env edits (conda is smart enough to use pydantic=1 if you constrain autodoc-pydantic=1, but pip is not, and it actually installs 1.0.0, hence the explicit pins) but they fail to deploy, hence the x. Possibly that's because it's a non-Ben PR.

loriab commented 1 year ago

Ok, I fixed the docs trouble. But the old manager + parsl tests (3 pythons) are all still running (>1h) and I notice that they used to finish in ~20 minutes (the pool and dask ones still do), so there might be something wrong there still. No output as of yet from the tests step.

loriab commented 1 year ago

The old manager + parsl are failing all pytests tests in 3h because psi4 is failing or the communication is failing to connect, so it retries over and over. I did go ahead and update the 1.8.1 psi4s off the psi4 channel to be pydantic v1/v2 compatible and copy over the v1/v2 qcel and qcng to the psi4 channel, but there's no change. the CI is pulling psi4 v1.7 anyways, and parsl is pulling the same psi4 as dask and pool, which are working just fine. So I don't know what else to do on this short of upgrading the whole env spec to v1.8.1 and c-f-based, and that seems too disruptive to do w/o consultation.

mattwthompson commented 9 months ago

Are those failures related to the changes in this patch? They don't obviously seem so, but I could be missing something.

loriab commented 9 months ago

Are those failures related to the changes in this patch? They don't obviously seem so, but I could be missing something.

If this was to me, no, I think this patch is perfectly harmless (and desirable). I think this needs a thorough rebase since the next->main transition. Or a cherry-pick or hand edit might be a better strategy.

mattwthompson commented 9 months ago

I would like this patch so I'll see if I can finish it up