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

Remove Python 3.8 from CI (#824) #826

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/actions/run-tests/action.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This code is part of a Qiskit project.
#
# (C) Copyright IBM 2021, 2023.
# (C) Copyright IBM 2021, 2024.
#
# This code is licensed under the Apache License, Version 2.0. You may
# obtain a copy of this license in the LICENSE.txt file in the root directory
Expand Down Expand Up @@ -36,7 +36,7 @@ runs:
if [ "${{ inputs.event-name }}" == "schedule" ] || [ "${{ inputs.run-slow }}" == "true" ]; then
export QISKIT_TESTS="run_slow"
fi
if [ "${{ inputs.os }}" == "ubuntu-latest" ] && [ "${{ inputs.python-version }}" == "3.8" ]; then
if [ "${{ inputs.os }}" == "ubuntu-latest" ] && [ "${{ inputs.python-version }}" == "3.9" ]; then
export PYTHON="coverage3 run --source qiskit_machine_learning --parallel-mode"
fi
stestr --test-path test run 2> >(tee /dev/stderr out.txt > /dev/null)
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/deploy-code.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This code is part of a Qiskit project.
#
# (C) Copyright IBM 2021, 2023.
# (C) Copyright IBM 2021, 2024.
#
# This code is licensed under the Apache License, Version 2.0. You may
# obtain a copy of this license in the LICENSE.txt file in the root directory
Expand All @@ -25,7 +25,7 @@ jobs:
id-token: write
strategy:
matrix:
python-version: [3.8]
python-version: [3.9]
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/deploy-docs.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This code is part of a Qiskit project.
#
# (C) Copyright IBM 2021, 2023.
# (C) Copyright IBM 2021, 2024.
#
# This code is licensed under the Apache License, Version 2.0. You may
# obtain a copy of this license in the LICENSE.txt file in the root directory
Expand All @@ -25,7 +25,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: [3.8]
python-version: [3.9]
steps:
- uses: actions/checkout@v4
with:
Expand Down
34 changes: 15 additions & 19 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest]
python-version: [3.8]
python-version: [3.9]
steps:
- name: Print Concurrency Group
env:
Expand Down Expand Up @@ -112,14 +112,14 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest]
python-version: [3.8, 3.9, '3.10', 3.11, 3.12]
python-version: [3.9, '3.10', 3.11, 3.12]
include:
- os: macos-latest
python-version: 3.8
python-version: 3.9
- os: macos-latest
python-version: 3.12
- os: windows-latest
python-version: 3.8
python-version: 3.9
- os: windows-latest
python-version: 3.12
# macos-14 is an Arm64 image
Expand Down Expand Up @@ -165,7 +165,7 @@ jobs:
run: |
coverage3 combine
mv .coverage ./ci-artifact-data/ml.dat
if: ${{ matrix.os == 'ubuntu-latest' && matrix.python-version == 3.8 }}
if: ${{ matrix.os == 'ubuntu-latest' && matrix.python-version == 3.9 }}
shell: bash
- uses: actions/upload-artifact@v4
with:
Expand All @@ -188,7 +188,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest]
python-version: [3.8, 3.12]
python-version: [3.9, 3.12]
steps:
- uses: actions/checkout@v4
with:
Expand Down Expand Up @@ -251,29 +251,25 @@ jobs:
# cd docs/_build/html
# mkdir artifacts
# tar -zcvf artifacts/tutorials.tar.gz --exclude=./artifacts .
# if: ${{ matrix.python-version == 3.8 && !startsWith(github.ref, 'refs/heads/stable') && !startsWith(github.base_ref, 'stable/') }}
# if: ${{ matrix.python-version == 3.9 && !startsWith(github.ref, 'refs/heads/stable') && !startsWith(github.base_ref, 'stable/') }}
# shell: bash
# - name: Run upload stable tutorials
# uses: actions/upload-artifact@v4
# with:
# name: tutorials-stable${{ matrix.python-version }}
# path: docs/_build/html/artifacts/tutorials.tar.gz
# if: ${{ matrix.python-version == 3.8 && !startsWith(github.ref, 'refs/heads/stable') && !startsWith(github.base_ref, 'stable/') }}
# if: ${{ matrix.python-version == 3.9 && !startsWith(github.ref, 'refs/heads/stable') && !startsWith(github.base_ref, 'stable/') }}
Deprecation_Messages_and_Coverage:
needs: [Checks, MachineLearning, Tutorials]
runs-on: ubuntu-latest
strategy:
matrix:
python-version: [3.8]
python-version: [3.9]
Comment on lines 262 to +267
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On thing to note, that at the point when we want to merge this PR we should update the branch rules such that 3.9 jobs, that replaced the prior 3.8 ones, are Required to pass and those old 3.8 ones are not any more.

While looking at the jobs, as we have had the new M1 jobs for some while, do we want to make them passing as Required for the merge too...

I was expecting changing python version here and in the "needs" jobs would run them with 3.9 directly, but they appear to pick up 3.8 instead. What would you suggest in this case?

Copy link
Member

@woodsp-ibm woodsp-ibm Aug 23, 2024

Choose a reason for hiding this comment

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

How the (Branch Protection) rules work is you select from the jobs that are run which are required - its configured. The config still has the old jobs, but since they are no longer run they show up as required but of course they will never pass since they are not run. What needs to happen is that the new jobs that we require to pass are selected/configured into the rule and the old ones removed. You can see on PRs that do not change the jobs that things are still correct since the jobs configured in the rules are the ones that are run. As this changes things we will need to update the rules. Its something we end up doing from time to time is jobs are changed, such as dropping a Python version or adding a new one. Changing the rules just needs to be coordinated with this PR being merged as the rules will affect other PRs from the moment its changed since the rule is for the branch the PR targets.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I understand, thanks @woodsp-ibm. Let's merge #827 first and then un-require the old jobs. In which file would you suggest changing the job configs?

steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- uses: actions/download-artifact@v4
with:
name: ubuntu-latest-3.8
path: /tmp/u38
- uses: actions/download-artifact@v4
with:
name: ubuntu-latest-3.9
Expand All @@ -292,16 +288,16 @@ jobs:
path: /tmp/u312
- uses: actions/download-artifact@v4
with:
name: macos-latest-3.8
path: /tmp/m38
name: macos-latest-3.9
path: /tmp/m39
- uses: actions/download-artifact@v4
with:
name: macos-latest-3.12
path: /tmp/m312
- uses: actions/download-artifact@v4
with:
name: windows-latest-3.8
path: /tmp/w38
name: windows-latest-3.9
path: /tmp/w39
- uses: actions/download-artifact@v4
with:
name: windows-latest-3.12
Expand All @@ -319,10 +315,10 @@ jobs:
shell: bash
- name: Combined Deprecation Messages
run: |
sort -f -u /tmp/u38/ml.dep /tmp/u39/ml.dep /tmp/u310/ml.dep /tmp/u311/ml.dep /tmp/u312/ml.dep /tmp/m38/ml.dep /tmp/m312/ml.dep /tmp/w38/ml.dep /tmp/w312/ml.dep /tmp/a310/ml.dep /tmp/a312/ml.dep || true
sort -f -u /tmp/u39/ml.dep /tmp/u310/ml.dep /tmp/u311/ml.dep /tmp/u312/ml.dep /tmp/m39/ml.dep /tmp/m312/ml.dep /tmp/w39/ml.dep /tmp/w312/ml.dep /tmp/a310/ml.dep /tmp/a312/ml.dep || true
shell: bash
- name: Coverage combine
run: coverage3 combine /tmp/u38/ml.dat
run: coverage3 combine /tmp/u39/ml.dat
shell: bash
- name: Upload to Coveralls
env:
Expand Down
4 changes: 2 additions & 2 deletions .mergify.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
queue_rules:
- name: automerge
conditions:
- check-success=Deprecation_Messages_and_Coverage (3.8)
- check-success=Deprecation_Messages_and_Coverage (3.9)

pull_request_rules:
- name: automatic merge on CI success and review
conditions:
- check-success=Deprecation_Messages_and_Coverage (3.8)
- check-success=Deprecation_Messages_and_Coverage (3.9)
- "#approved-reviews-by>=1"
- label=automerge
- label!=on hold
Expand Down
2 changes: 2 additions & 0 deletions .pylintdict
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ elif
endian
entangler
enum
eol
eps
estimatorqnn
et
Expand Down Expand Up @@ -352,6 +353,7 @@ o'brien
objval
observables
oct
october
olson
onboarding
onodera
Expand Down
2 changes: 1 addition & 1 deletion constraints.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
numpy>=1.20,<2.0
ipython<8.13;python_version<'3.9'
ipython<8.13;python_version<'3.10'
Copy link
Member

Choose a reason for hiding this comment

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

Is this change due to bumping the version we use from 3.8 to 3.9 and we just did not have the constraint right before in that it really applied to more than just 3.8

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My understanding of <'3.9' is that ipython will pick 3.8, which is the only allowed version strictly lower than 3.9. So, to allow 3.9 I raised <'3.9' to <'3.10'. Is this correct?

Copy link
Member

Choose a reason for hiding this comment

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

This is a constraint (limits) over what is installed when it's needed to be installed. So that basically says if ipython is installed it should be less than version < 8.13 but with the caveat that this only for when the python version its being installed into is less than that value. So before it would have installed some version of ipython < 8.13 when using 3.8 and was not constrained in other cases so it would have used whatever version it got resolved to, which is likely much newer. Now with bumping the Python version there it will be constraining it to < 8.13 in Python 3.9, which it was not before. But then I do not know which jobs exactly end up needing/using it - I imagine its for the notebook ones - which we would have run on 3.8 (and 3.12) before but not 3.9. You could try and see without that constraint.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I'll try out the tests without ipython<8.13;python_version<'3.10' in the next commit.

nbconvert<7.14 # workaround https://github.com/jupyter/nbconvert/issues/2092

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ build-backend = "setuptools.build_meta"

[tool.black]
line-length = 100
target-version = ['py38', 'py39', 'py310', 'py311']
target-version = ['py39', 'py310', 'py311', 'py312']
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
deprecations:
edoaltamura marked this conversation as resolved.
Show resolved Hide resolved
- |
Removed support for using Qiskit Machine Learning with Python 3.8, reflecting
the upcoming EOL of Python 3.8 in October 2024.
3 changes: 1 addition & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
"Operating System :: MacOS",
"Operating System :: POSIX :: Linux",
"Programming Language :: Python :: 3 :: Only",
"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
Expand All @@ -68,7 +67,7 @@
packages=setuptools.find_packages(include=['qiskit_machine_learning','qiskit_machine_learning.*']),
install_requires=REQUIREMENTS,
include_package_data=True,
python_requires=">=3.8",
python_requires=">=3.9",
extras_require={
'torch': ["torch"],
'sparse': ["sparse"],
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[tox]
# Sets this min.version because of differences with env_tmp_dir env.
minversion = 4.0.2
envlist = py38, py39, py310, py311, py312, lint, gpu, gpu-amd
envlist = py39, py310, py311, py312, lint, gpu, gpu-amd
skipsdist = True

[testenv]
Expand Down
Loading