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

Dependencies: Restrict Python to strictly below 3.13 #6603

Closed
wants to merge 1 commit into from

Conversation

agoscinski
Copy link
Contributor

Recstrict Python version ito strictly below 3.13 until we have resolved the dependency issues in PR #6600.

Recstrict Python version ito strictly below 3.13 until we have resolved
the dependency issues in PR aiidateam#6600.
@agoscinski agoscinski requested a review from sphuber November 4, 2024 16:36
Copy link

codecov bot commented Nov 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.87%. Comparing base (ef60b66) to head (067a482).
Report is 185 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6603      +/-   ##
==========================================
+ Coverage   77.51%   77.87%   +0.37%     
==========================================
  Files         560      567       +7     
  Lines       41444    42120     +676     
==========================================
+ Hits        32120    32796     +676     
  Misses       9324     9324              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sphuber
Copy link
Contributor

sphuber commented Nov 4, 2024

Not saying we shouldn't do this, but is this really necessary? Our own code is compatible with Python 3.13. It is just that certain dependencies aren't. Adding this limitation is not going to do much then, right? Especially not if we are not going to release it soon. By the time we would release it we will probably be compatible with 3.13 anyway and we would have to remove it again. So far we have never imposed an explicit limit and it has never caused issues. I am also not aware that this is recommended as a best practice by the PyPA.

@danielhollas
Copy link
Collaborator

Yeah, I think this is actually discouraged, to the point where for example uv ignores maximum python version limits when installing dependencies (I think).

@agoscinski
Copy link
Contributor Author

Yeah, I think this is actually discouraged, to the point where for example uv ignores maximum python version limits when installing dependencies (I think).

Read the same astral-sh/uv#8374 (comment)

There is a huge discussion about it, but I don't see any result comig from it
https://discuss.python.org/t/requires-python-upper-limits/12663/80
The metadata field is used by some like pyreads https://pyreadiness.org/3.13/ but the problem that you install Python 3.13 because it is the newest and then AiiDA crashes remains to me. I don't see a simple solution for this.

@agoscinski
Copy link
Contributor Author

Since we agreed for the release to not include it, this can be closed. If this comes up again as a general discussion one can open an issue.

@agoscinski agoscinski closed this Nov 6, 2024
@danielhollas
Copy link
Collaborator

danielhollas commented Nov 6, 2024 via email

@sphuber
Copy link
Contributor

sphuber commented Nov 6, 2024

The main issue is, even if you did include it in the release it would be
kind of pointless, since the resolver would simply backtrack to the
previous version that did not include the constraint. :-/

☝️ That is exactly it.

@sphuber
Copy link
Contributor

sphuber commented Nov 6, 2024

Another interesting and well laid out point of view just for reference: https://iscinumpy.dev/post/bound-version-constraints/

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.

3 participants