-
Notifications
You must be signed in to change notification settings - Fork 208
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
Pylint and Mypy fixes to solve CI errors. Remove Python 3.8 support #1374
Conversation
Pull Request Test Coverage Report for Build 12931707835Details
💛 - Coveralls |
Looks fine to me overall. One comment I have about the title and text is that this does not deprecate 3.8 support it removes it completely. Deprecation keeps support but issues warnings about the intent to remove it in an upcoming version. Since its changed the package from the user perspective in that regard the PR should have a release note stating that 3.8 support has been removed such that when a release is generated, including this, that this comes out in the release notes documentation. See https://github.com/qiskit-community/qiskit-nature/blob/main/CONTRIBUTING.md#release-notes for creating release notes (you can see examples in past PRs too if you need any). I see you noted networkx requirement needed updating for numpy 2. I cannot recall any longer why it was pinned - I assume some compatibility related problem. Just noting that I think CI will install the latest unless constrained otherwise so maybe the entry in requirements-dev is not strictly needed. Commenting as I see the main requirements.txt file whose contents affect a normal install state numpy >=1.17 such that I guess if a user has an older numpy than 2.0 it still works. (I checked qiskit just because its a major dependency and it has the same min version) |
I will note the branch rules here will need updating such that new jobs are made Required as needed and older jobs, no longer run, get removed from being Required. This can be done just ahead of this being merged so the rules match what is needed to pass this given the CI changes here. |
@woodsp-ibm I changed the name to reflect the removal of Python 3.8 and added a release note. I have also removed It should be all good now, I am just waiting for approval and your update on the branch rules. |
@ftroisi FYI I see the nightly scheduled CI, that was run after this was merged, failed in unbuntu 3.12 with psi4, though it had passed here. You can see the CI runs under "Actions" its here https://github.com/qiskit-community/qiskit-nature/actions/runs/12960770104 Not sure if that was just an intermittent and/or related to the psi4 errors you had before. Update: I see last nights (Sat 25th thru Sun) scheduled CI passed. So I guess we have to see if the failure re-occurs or not. |
@woodsp-ibm Thanks for the message, the error was about a temp folder which could be found ( |
Summary
This PR originates from #1371, and fixes
pylint
andmypy
errors in the CI. Also, the packagenetworkx
had to be updated to supportnumpy 2.x
.Since tests must pass, PR #1371 has been incorporated in this PR. Thanks to @iyanmv for opening that PR.
Details and comments
Fixes: #1370
Note that since MacOS 14, github-hosted runners do not have Conda bundled. Thus, on the
macos-latest
pipeline, we have to install Conda (conda-incubator/setup-miniconda#344). A sincere thanks to @EndBug for the discussion on this topic.Python 3.8 reached EOL last year. Since there were
pylint
errors due to this old version andqiskit
no longer supports Python 3.8, this PR deprecates Python 3.8 support