-
Notifications
You must be signed in to change notification settings - Fork 192
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
CI: Support for Python 3.9 #4301
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #4301 +/- ##
===========================================
+ Coverage 79.49% 79.50% +0.01%
===========================================
Files 482 482
Lines 35327 35327
===========================================
+ Hits 28080 28082 +2
+ Misses 7247 7245 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Just to note that Python 3.9 was officially released today. Should probably wait for our dependencies to release compatible versions, but hopefully we can also soon add support |
I'll update this PR. |
Blocked by actions/setup-python#148 . |
Standard pip install succeeds now, installation with extras still fails. Python 3.9 is not available via conda channels yet, but should be shortly. |
A Python 3.9 conda environment can now be created via the conda-forge channel, but there are still many conflicts, likely because upstream packages are not available yet for Python 3.9. |
The pip install with extras is currently blocked by: nucleic/kiwi#92 |
Just realized that all of this is also completely dependent on our own maintained dependencies releasing a Python 3.9 compatible version: |
The conda install now passes, pip install with extensions still failing, because of kiwisolver: nucleic/kiwi#92 A release of a Python 3.9 compatible version is promised for by the end of the week. |
The pip install with and without extras succeeds now, but the conda installation no longer passes (go figure).. |
I am bit surprised that it passes at all to be honest. We haven't released a Python 3.9 compatible version for |
Well thats because of your loose python pinnings 😜 |
A well, fair enough. I forgot about that, I was thinking whether |
No, I've been guilty of it as well, but I guess you should really pin |
same with this repo actually: Line 10 in 9ff07c1
|
Isn't there a tradeoff? Clearly by adding an upper limit, you don't run the risk that the package will break for a new Python release. The same would happen with an upper limit, there it would just not even install and so the end result is the same, that package cannot be used in the new version. However, the advantage of not adding the upper limit is that there is a chance (and it being minor versions, they should be forwards compatible and therefore guaranteed) that the existing package simply works with the new version. This means upstream does not have to wait for a release. Imagine if every library put an upper limit. Each environment would then have to wait for everything downstream to have released a new version before it could do anything, massively slowing down migration to new Python releases. I have never thought or read about this so this is an honest question on what the "best" practice is here. |
Yeh neither have I really. |
I think it depends a bit on the complexity of the library/application. In the case of aiida-core it might make sense to set an upper limit and only increase it once it has been properly tested for that newer version. However, I would probably advocate for the "opposite" approach and only explicitly limit if incompatibilities are found. |
9efad20
to
60b1523
Compare
And add requirements file for Python 3.9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @csadorf . Mostly have a few questions, some of which are in the code. Another one is that I am a bit surprised the tests actually pass. I have been seeing deprecation warnings coming from reentry
for a while which still uses collections.Mapping
which is deprecated and no longer works ion 3.9 in favor of collections.abc.Mapping
. I guess that part of the code is not actually used in our code paths. @ltalirz I also found that reentry
is still on dropd's fork. Should we move this to aiidateam
and start to add other core members as maintainers in addition to yourself?
@sphuber I addressed your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, thanks @csadorf
Thx! I'll wait for all tests to pass and then squash-merge. Edit: Sorry, just saw your comment above, please go ahead then. |
According to the Python 3.9 RC release notes:
I will track preparations to support Python 3.9 on this branch. The CI workflow for this branch is expected to fail until further notice.