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.

Sign up for GitHub

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

chore: support Python 3.12 #226

Merged
merged 5 commits into from
Jul 24, 2024
Merged

chore: support Python 3.12 #226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a8e68af
chore: support Python 3.12
SdgJlbl Jun 28, 2024
8147c7f
chore: bump dep versions for benchmark workflow
SdgJlbl Jul 23, 2024
34db729
fix: add ignore for in the doc
SdgJlbl Jul 23, 2024
d67e0bc
fix: substratools and substra versions
SdgJlbl Jul 23, 2024
d68f6a7
chore: run benchmark on all Python version
SdgJlbl Jul 24, 2024
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
10 changes: 5 additions & 5 deletions .github/workflows/main-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ jobs:
repository: substra/substra
path: substra

- uses: actions/[email protected]
id: cache
with:
path: ${{ env.pythonLocation }}
key: ${{ runner.os }}-${{ env.pythonLocation }}-pip-${{ hashFiles('substrafl/pyproject.toml') }}-${{ hashFiles ('substrafl/docs/requirements.txt') }}-${{ hashFiles('substrafl/benchmark/camelyon/requirements.txt') }}
# - uses: actions/[email protected]
# id: cache
# with:
# path: ${{ env.pythonLocation }}
# key: ${{ runner.os }}-${{ env.pythonLocation }}-pip-${{ hashFiles('substrafl/pyproject.toml') }}-${{ hashFiles ('substrafl/docs/requirements.txt') }}-${{ hashFiles('substrafl/benchmark/camelyon/requirements.txt') }}

- name: Install package
run: |
Expand Down
13 changes: 9 additions & 4 deletions .github/workflows/validate-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-22.04]
python: ["3.9", "3.10", "3.11"]
python: ["3.9", "3.10", "3.11", "3.12"]

steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -74,7 +74,7 @@ jobs:

- uses: actions/setup-python@v5
with:
python-version: "3.10"
python-version: "3.11"
thbcmlowk marked this conversation as resolved.
Show resolved Hide resolved

- uses: actions/[email protected]
id: cache
Expand Down Expand Up @@ -105,11 +105,16 @@ jobs:
make clean html

benchmark:
runs-on: ubuntu-22.04
name: benchmark-${{ matrix.python }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-22.04]
python: ["3.9", "3.10", "3.11", "3.12"]
steps:
- uses: actions/setup-python@v5
with:
python-version: 3.11
python-version: 3.12
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we rather rely on a python matrix there too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean, running the benchmark with all versions of Python?
I guess it wasn't done for cost effectiveness reasons, but it can be changed, indeed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's what I meant! (cf: comment on benchmark requirements)


- uses: actions/checkout@v4
with:
Expand Down
8 changes: 4 additions & 4 deletions benchmark/camelyon/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
tqdm==4.66.3
numpy==1.24.3
scikit-learn==1.5.0
torch==2.0.1
torchvision==0.15.2
numpy==1.26.4
scikit-learn==1.5.1
torch==2.3.0
torchvision==0.18.0
--extra-index-url https://download.pytorch.org/whl/cpu
thbcmlowk marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 3 additions & 3 deletions benchmark/camelyon/workflows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ def substrafl_fed_avg(
)

pypi_dependencies = [
"numpy==1.24.3",
"torch==2.0.1",
"scikit-learn==1.3.1",
"numpy==1.26.4",
"torch==2.3.0",
"scikit-learn==1.5.1",
]
if not torch_gpu:
pypi_dependencies += ["--extra-index-url https://download.pytorch.org/whl/cpu"]
Expand Down
1 change: 1 addition & 0 deletions changes/226.added
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Python 3.12 support
2 changes: 1 addition & 1 deletion docs/api/algorithms.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ TorchFedAvgAlgo
TorchScaffoldAlgo
-----------------

.. automodule:: substrafl.algorithms.pytorch.torch_scaffold_algo
.. autoclass:: substrafl.algorithms.pytorch.torch_scaffold_algo.TorchScaffoldAlgo
:private-members: _local_train, _local_predict, _scaffold_parameters_update, _save_predictions
:inherited-members:

Expand Down
2 changes: 2 additions & 0 deletions docs/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@
("py:class", "FieldInfo"),
("py:class", "ConfigDict"),
("py:class", "ComputedFieldInfo"),
("py:class", "CUpdateRule"),
("py:class", "substrafl.algorithms.pytorch.torch_scaffold_algo.CUpdateRule"),
]

html_css_files = [
Expand Down
5 changes: 3 additions & 2 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ classifiers = [
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12",
]
dynamic = ["version"]
readme = "README.md"
dependencies = [
"numpy>=1.24,<2.0",
"cloudpickle>=1.6.0",
"substra~=0.53.0",
"substratools~=0.21.4",
"substra~=0.54.0a1",
"substratools~=0.22.0a2",
Comment on lines +25 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

To recap:

  • substratools in its alpha version is needed because we pip install it in the docker template
  • substra in its alpha version is needed because?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can try to make it work without the alphas, if we think it's worth it to invest the extra time 🤷

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is that we do need the substratools release for the python setup in docker. I would like to understand why do we need the substra pre-release. If you don't know it's ok, feel free to resolve the comment.

"pydantic>=2.3.0,<3.0",
"pip>=21.2",
"tqdm",
Expand Down
2 changes: 1 addition & 1 deletion substrafl/remote/register/register.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
# minimal and maximal values of Python 3 minor versions supported
# we need to store this as integer, else "3.11" < "3.9" (string comparison)
MINIMAL_PYTHON_VERSION = 9 # 3.9
MAXIMAL_PYTHON_VERSION = 11 # 3.11
MAXIMAL_PYTHON_VERSION = 12 # 3.12

_DEFAULT_BASE_DOCKER_IMAGE = "python:{python_version}-slim"

Expand Down
Loading