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.

Sign up for GitHub

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

Jump to bottom

Dependencies: Update pydantic~=2.4 #977

Merged
merged 1 commit into from
Dec 22, 2023
Merged

Dependencies: Update pydantic~=2.4 #977

merged 1 commit into from
Dec 22, 2023

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Oct 28, 2023

The requirement for pydantic was pinned to v1 since v2 has a lot of backwards incompatible changes and it is difficult to provide a version that is compatible with both versions.

As of v2.5.0, aiida-core also directly depends on pydantic and it requires ~=2.4, so here we apply the same requirement. The deprecated code is replaced.

@sphuber sphuber requested a review from bastonero October 28, 2023 18:40
@mbercx
Copy link
Member

mbercx commented Nov 12, 2023

Thanks @sphuber! Quick question: if we merge this, the next aiida-quantumespresso release would only be compatible with aiida-core~=2.5, correct? Don't we have to update the aiida-core dependency then? And if we do, do we still need an explicit dependency on pydantic here?

@sphuber
Copy link
Contributor Author

sphuber commented Nov 17, 2023

Quick question: if we merge this, the next aiida-quantumespresso release would only be compatible with aiida-core~=2.5, correct?

Indirectly, yes. But you can put it also the other way around. Without this fix, aiida-quantumespresso will be incompatible with aiida-core>=2.5. Given the big changes between pydantic v1 and v2, it is almost impossible to have compatibility for both versions at the same time, and so our hand is pretty much forced. But aiida-core is forward compatible in the minor versions, so it shouldn't be a problem. Everyone should be able to update without issues.

Don't we have to update the aiida-core dependency then?

Technically that is not really our responsiblity. Package managers like pip will figure this out.

And if we do, do we still need an explicit dependency on pydantic here?

I would say yes absolutely, because we directly use it. So we should specify what requirements aiida-quantumespresso has on that package

@rikigigi rikigigi mentioned this pull request Dec 1, 2023
Merged
mbercx
mbercx previously approved these changes Dec 22, 2023
View reviewed changes
Copy link
Member

@mbercx mbercx left a comment

Choose a reason for hiding this comment

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

No notes, thanks @sphuber!

@sphuber sphuber force-pushed the fix/pydantic branch 2 times, most recently from f0dd026 to 0a94288 Compare December 22, 2023 15:47
@sphuber
Dependencies: Update pydantic~=2.4
5fb5833 
The requirement for `pydantic` was pinned to `v1` since `v2` has a lot
of backwards incompatible changes and it is difficult to provide a
version that is compatible with both versions.

As of `v2.5.0`, `aiida-core` also directly depends on `pydantic` and it
requires `~=2.4`, so here we apply the same requirement. The deprecated
code is replaced.

As a side effect, a test for `PwCalculation` started failing. It was
testing that no warnings were raised for specific inputs, but some
warnings _were_ being raised. These were not the warnings tested for in
the tests though, but raised by SQLAlchemy. AiiDA v2.5 upgraded to
SQLAlchemy v2 which as of v2.0.19 started emitting a warning. This is
ignored by a filter in `aiida-core`, but this is made undone by `pytest`.
The warning is filtered again in the `pyproject.toml` config for
`pytest` but this is not considered by `pytest.warns`. Therefore, the
warnings in `PwCalculation` are turned into the more specific
`UserWarning` such that the test can explicitly check for those.
@sphuber sphuber merged commit 740e0be into main Dec 22, 2023
13 checks passed
@sphuber sphuber deleted the fix/pydantic branch December 22, 2023 16:00
@unkcpz unkcpz mentioned this pull request Jan 11, 2024
Merged
@unkcpz
Copy link
Member

unkcpz commented Jan 12, 2024

@sphuber @mbercx Plan to make a release with this commit? We want to give it a test on AiiDAlab.

@sphuber
Copy link
Contributor Author

sphuber commented Jan 12, 2024

I think we are long due for a release. Marnik had planned on preparing it but hasn't come around to it. I think I will start preparing it now and have him review it

bastonero pushed a commit to bastonero/aiida-quantumespresso that referenced this pull request Jan 6, 2025
@sphuber @bastonero
Dependencies: Update pydantic~=2.4 (aiidateam#977)
f6a1233 
The requirement for `pydantic` was pinned to `v1` since `v2` has a lot
of backwards incompatible changes and it is difficult to provide a
version that is compatible with both versions.

As of `v2.5.0`, `aiida-core` also directly depends on `pydantic` and it
requires `~=2.4`, so here we apply the same requirement. The deprecated
code is replaced.

As a side effect, a test for `PwCalculation` started failing. It was
testing that no warnings were raised for specific inputs, but some
warnings _were_ being raised. These were not the warnings tested for in
the tests though, but raised by SQLAlchemy. AiiDA v2.5 upgraded to
SQLAlchemy v2 which as of v2.0.19 started emitting a warning. This is
ignored by a filter in `aiida-core`, but this is made undone by `pytest`.
The warning is filtered again in the `pyproject.toml` config for
`pytest` but this is not considered by `pytest.warns`. Therefore, the
warnings in `PwCalculation` are turned into the more specific
`UserWarning` such that the test can explicitly check for those.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbercx mbercx mbercx left review comments

@bastonero bastonero Awaiting requested review from bastonero

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sphuber @mbercx @unkcpz