-
Notifications
You must be signed in to change notification settings - Fork 187
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: manually install numpy==1.19.4
to prevent release candidate
#4615
CI: manually install numpy==1.19.4
to prevent release candidate
#4615
Conversation
Thinking about this, I will actually have to base this of |
Codecov Report
@@ Coverage Diff @@
## release/1.5.1 #4615 +/- ##
=================================================
- Coverage 79.51% 79.50% -0.00%
=================================================
Files 482 482
Lines 35320 35320
=================================================
- Hits 28080 28078 -2
- Misses 7240 7242 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Does this relate to a particular open issue on pymatgen, so I can go and voice my displeasure lol Also, should we add a note on this somewhere in the documentation; in the install or troubleshooting section? |
No current one, but we have dealt with this problem already at least 4 times. Each time For reference the commits I could quickly find that addressed the exact same problem in the past:
Would be a good idea since it is happening often. At least by know I am able to recognize the problem almost instantly and add the workaround in place without wasting a lot of precious time, but we should document this. Still other developers should then still be aware of this phenomenon. In any case, given that it concerns |
78c5832
to
61da6bf
Compare
On a related note @csadorf we can remove the 2020-resolver now:
|
Will this not then affect people running I was actually just trying to reproduce this locally, but then saw that its not an issue for Macs, because there is a pymatgen wheel (materialsproject/pymatgen#1520 (comment)). Macs are clearly the best 😝 |
Yes, it does: https://github.com/aiidateam/aiida-core/runs/1496526194?check_suite_focus=true It is quite frustrating that this is problem is arising yet again. Those are the actions I would recommend:
For clarification, restricting our dependencies to versions of pymatgen that support the same Python versions that we support with markers does not solve the problem, correct? |
Indeed, this does not solve the problem, because the limitations we impose through the |
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.
I've made some suggestions on how to make this work-around fix a bit less intrusive, but overall it is of course fine, at least for the ci-code workflow. We can't have this impede our dev work. I'm a bit more hesitant to accept this for the test-install workflow.
Either way, I want to be pragmatic about this, so the fix is fine, even without my proposed changes, albeit I would strongly recommend to at least accept the in-line comments.
@@ -92,6 +92,7 @@ jobs: | |||
|
|||
- name: Install aiida-core | |||
run: | | |||
pip install numpy==1.19.4 |
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.
Let's at least ensure that the version from the requirements files is installed and add a comment as to why this work-around is necessary:
pip install numpy==1.19.4 | |
# Work-around issue caused by pymatgen's setup process, which will install the latest | |
# numpy version (including release candidates) regardless of our actual specification: | |
pip install `grep 'numpy==' requirements/requirements-py-${{ matrix.python-version }}.txt` |
This is the usual problem that occurs when `numpy` releases a candidate release. Even though we specify limits to not install this, `pymatgen` installs it as a dependency in their build process where limits are ignored and so the latest release gets installed. In this case `numpy==1.20.0rc1` is getting installed and that dropped support for Python 3.6 and so our builds on that version brick. The workaround is to manually install the compatible version, in this case `numpy==1.19.4` before installing `aiida-core`. This way `pymatgen` will simply use that in their build process.
78fe8e6
to
f057efb
Compare
For reference: the final solution that got merged here was not ideal. It hardcoded |
Is this not expected behavior, and the responsibility of setuptools to make that determination?
This would definitely be appreciated and in the spirit of open source. As mentioned more specifically in the #4614 issue and the referenced issue in pymatgen, we have been unable to identify or reproduce the issue. More broadly, for creating re-producible builds, I can recommend poetry since pip alone can often be very brittle. One thing that also comes to mind is that pip 20.3 has also made significant changes in how its dependency resolver works, so maybe that could be causing issues. |
@mkhorton thanks a lot for following up on this, really appreciate it. I responded to the issue over at pymatgen with some additional information on how we encounter the problem and where I think the problem comes from, but it is still not entirely clear. I propose we continue the discussion there and see if we can nail down the origin of the problem and hopefully come up with a solution. |
Fixes #4614
This is the usual problem that occurs when
numpy
releases a candidaterelease. Even though we specify limits to not install this,
pymatgen
installs it as a dependency in their build process where limits are
ignored and so the latest release gets installed. In this case
numpy==1.20.0rc1
is getting installed and that dropped support forPython 3.6 and so our builds on that version brick.
The workaround is to manually install the compatible version, in this
case
numpy==1.19.4
before installingaiida-core
. This waypymatgen
will simply use that in their build process.