From ea0f33ed0bf4e9901166223378e1a51419fdca73 Mon Sep 17 00:00:00 2001 From: ThibaultFy Date: Thu, 3 Oct 2024 15:11:59 +0200 Subject: [PATCH] chore: erase traces of substratools Signed-off-by: ThibaultFy --- .github/workflows/main-check.yml | 7 ------- .github/workflows/validate-pr.yml | 18 ------------------ .github/workflows/windows-subprocess-check.yml | 7 ------- benchmark/camelyon/README.md | 1 - benchmark/camelyon/common/utils.py | 2 -- .../camelyon/pure_substrafl/assets/opener.py | 2 +- docs/technical/task_execution.rst | 10 ++-------- pyproject.toml | 1 - substrafl/dependency/manage_dependencies.py | 4 +--- substrafl/dependency/schemas.py | 9 +++------ substrafl/exceptions.py | 6 +----- substrafl/experiment.py | 10 ++++------ substrafl/model_loading.py | 3 +-- substrafl/nodes/utils.py | 5 ++--- substrafl/remote/register/register.py | 10 +++------- substrafl/remote/substratools_methods.py | 4 ++-- tests/assets_factory.py | 9 +-------- tests/dependency/test_manage_dependencies.py | 10 +++++----- tests/remote/register/test_register.py | 3 --- tests/test_model_loading.py | 2 -- 20 files changed, 26 insertions(+), 97 deletions(-) diff --git a/.github/workflows/main-check.yml b/.github/workflows/main-check.yml index 3cea81ed..b3631dae 100644 --- a/.github/workflows/main-check.yml +++ b/.github/workflows/main-check.yml @@ -32,12 +32,6 @@ jobs: sudo rm -rf "/usr/local/share/boost" sudo rm -rf "$AGENT_TOOLSDIRECTORY" - - name: Checkout pyconfig from a private repos - uses: actions/checkout@v4 - with: - repository: substra/substra-tools - path: substratools - - name: Checkout pyconfig from a private repos uses: actions/checkout@v4 with: @@ -55,7 +49,6 @@ jobs: pip install --upgrade pip pip install --upgrade -e substrafl[dev] pip install --upgrade -e substra - pip install --upgrade -e substratools - name: Run the slow local tests run: | diff --git a/.github/workflows/validate-pr.yml b/.github/workflows/validate-pr.yml index 985d3813..3e545d01 100644 --- a/.github/workflows/validate-pr.yml +++ b/.github/workflows/validate-pr.yml @@ -40,11 +40,6 @@ jobs: cd substrafl pre-commit run --all-files - - uses: actions/checkout@v4 - with: - repository: substra/substra-tools - path: substratools - - uses: actions/checkout@v4 with: repository: substra/substra @@ -54,7 +49,6 @@ jobs: run: | pip install --upgrade -e substrafl[dev] pip install --upgrade -e substra - pip install --upgrade -e substratools - name: Run the fast local tests run: | @@ -87,15 +81,9 @@ jobs: repository: substra/substra path: substra - - uses: actions/checkout@v4 - with: - repository: substra/substra-tools - path: substratools - - name: Install Dependencies run: | pip install --upgrade ./substra - pip install --upgrade -e ./substratools pip install --upgrade .[dev] pip install --upgrade -r docs/requirements.txt @@ -120,11 +108,6 @@ jobs: with: path: substrafl - - uses: actions/checkout@v4 - with: - repository: substra/substra-tools - path: substratools - - uses: actions/checkout@v4 with: repository: substra/substra @@ -147,7 +130,6 @@ jobs: run: | pip install --upgrade -e substrafl[dev] pip install --upgrade -e substra - pip install --upgrade -e substratools pip install --upgrade -r substrafl/benchmark/camelyon/requirements.txt - name: Run the local benchmark diff --git a/.github/workflows/windows-subprocess-check.yml b/.github/workflows/windows-subprocess-check.yml index 2c939ac5..a20ca862 100644 --- a/.github/workflows/windows-subprocess-check.yml +++ b/.github/workflows/windows-subprocess-check.yml @@ -21,12 +21,6 @@ jobs: with: python-version: ${{ matrix.python }} - - name: Checkout pyconfig from a private repos - uses: actions/checkout@v4 - with: - repository: substra/substra-tools - path: substratools - - name: Checkout pyconfig from a private repos uses: actions/checkout@v4 with: @@ -43,7 +37,6 @@ jobs: run: | pip install --upgrade -e "substrafl[dev]" pip install --upgrade -e substra - pip install --upgrade -e substratools - name: Run the subprocess tests run: | diff --git a/benchmark/camelyon/README.md b/benchmark/camelyon/README.md index 0976a930..bf745ec6 100644 --- a/benchmark/camelyon/README.md +++ b/benchmark/camelyon/README.md @@ -136,7 +136,6 @@ After each run the [result file](./results/results.json) is populated with a new "seed": 42, "substra_version": "0.45.0", "substrafl_version": "0.38.0", - "substratools_version": "0.20.0" } } ``` diff --git a/benchmark/camelyon/common/utils.py b/benchmark/camelyon/common/utils.py index b971fe11..2a11cd77 100644 --- a/benchmark/camelyon/common/utils.py +++ b/benchmark/camelyon/common/utils.py @@ -4,7 +4,6 @@ from pathlib import Path import substra -import substratools import substrafl @@ -22,7 +21,6 @@ def parse_params() -> dict: "learning_rate": 0.01, "substrafl_version": substrafl.__version__, "substra_version": substra.__version__, - "substratools_version": substratools.__version__, } parser = argparse.ArgumentParser("Default parser.", formatter_class=argparse.ArgumentDefaultsHelpFormatter) diff --git a/benchmark/camelyon/pure_substrafl/assets/opener.py b/benchmark/camelyon/pure_substrafl/assets/opener.py index f46c491f..98009239 100644 --- a/benchmark/camelyon/pure_substrafl/assets/opener.py +++ b/benchmark/camelyon/pure_substrafl/assets/opener.py @@ -3,7 +3,7 @@ from typing import List import numpy as np -import substratools as tools +from substra import tools logger = logging.getLogger(__name__) diff --git a/docs/technical/task_execution.rst b/docs/technical/task_execution.rst index 367d9a5b..b374b437 100644 --- a/docs/technical/task_execution.rst +++ b/docs/technical/task_execution.rst @@ -24,14 +24,11 @@ Base Dockerfile ^^^^^^^^^^^^^^^^ The Dockerfile describes the commands to run to create a container with all the needed dependencies. -Its base image is the substratools Docker image, accessible from the private Owkin docker registry (which is a google container registry). The base image is chosen following two criteria: - the version of Python is the same as the one the code is run with, to satisfy cloudpickle requirements -- the version of substratools is the version of the substratools installed in the Python environment (can be - overriden, see below). If the version is inferior to 0.10.0, we use 0.10.0 as the name of the substratools images - changed. +- the usage of GPU or not This means that: @@ -48,7 +45,6 @@ Substrafl dependencies Substrafl needs the following libraries to be installed in the container: -- substratools - substra - substrafl @@ -59,7 +55,7 @@ Substra. There are two modes: the **release mode** and the **editable mode**, chosen with the ``editable_mode`` parameter in the ``dependency`` argument. -In **release mode**, Substrafl downloads the wheels of substra and substrafl (substratools is already installed) from +In **release mode**, Substrafl downloads the wheels of substra and substrafl from the private Owkin PyPi and copies them to the Docker image. The download is made through a subprocess, and it needs pip to be configured to access Owkin's PyPi. @@ -73,8 +69,6 @@ Substrafl is executed with. The script goes through each library, and: Then copy the wheel to the Docker image. This is not the preferred method as it can lead to difficulties of knowing which version was used: there may be local changes to the code. -Please note that in editable mode substratools is re-installed in the image. - User dependencies ^^^^^^^^^^^^^^^^^^ diff --git a/pyproject.toml b/pyproject.toml index 94513085..5c146c64 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,7 +22,6 @@ dependencies = [ "numpy>=1.24", "cloudpickle>=1.6.0", "substra~=0.54.0", - "substratools~=0.22.0", "pydantic>=2.3.0,<3.0", "pip>=21.2", "tqdm", diff --git a/substrafl/dependency/manage_dependencies.py b/substrafl/dependency/manage_dependencies.py index ff5bea7d..770f317e 100644 --- a/substrafl/dependency/manage_dependencies.py +++ b/substrafl/dependency/manage_dependencies.py @@ -97,7 +97,7 @@ def local_lib_wheels(lib_modules: List[ModuleType], *, dest_dir: Path) -> List[s lib_name = lib_module.__name__ wheel_name = f"{lib_name}-{lib_module.__version__}-py3-none-any.whl" - # if the right version of substra or substratools is not found, it will search if they are already + # if the right version of substra is not found, it will search if they are already # installed in 'dist' and take them from there. # sys.executable takes the current Python interpreter instead of the default one on the computer @@ -105,8 +105,6 @@ def local_lib_wheels(lib_modules: List[ModuleType], *, dest_dir: Path) -> List[s [ "--find-links", dest_dir.parent / "substra", - "--find-links", - dest_dir.parent / "substratools", ] if lib_name == "substrafl" else [] diff --git a/substrafl/dependency/schemas.py b/substrafl/dependency/schemas.py index d2c1378b..88eedbd3 100644 --- a/substrafl/dependency/schemas.py +++ b/substrafl/dependency/schemas.py @@ -8,7 +8,6 @@ from typing import Optional import substra -import substratools from pydantic import BaseModel from pydantic import ConfigDict from pydantic import Field @@ -31,7 +30,7 @@ class Dependency(BaseModel): **local-worker** or with **tmp_substrafl** as prefix are ignored during the installation. Args: - editable_mode (bool): If set to False, substra, substrafl and substratools used in the + editable_mode (bool): If set to False, substra and substrafl used in the Dockerfiles submitted to Substra platform will be taken from pypi. If set to True, it will be the one installed in editable mode from your python environment. Defaults to False. @@ -177,8 +176,7 @@ def _compute(self, dest_dir: Path) -> None: └── substrafl_internal ├── dist │ ├── substra-0.44.0-py3-none-any.whl - │ ├── substrafl-0.36.0-py3-none-any.whl - │ └── substratools-0.20.0-py3-none-any.whl + │ └── substrafl-0.36.0-py3-none-any.whl ├── local_dependencies │ └── local-module-1.6.1-py3-none-any.whl ├── requirements.in # only if compile set to True @@ -195,8 +193,7 @@ def _compute(self, dest_dir: Path) -> None: lib_modules=[ substrafl, substra, - substratools, - ], # We reinstall substratools in editable mode to overwrite the installed version + ], dest_dir=substra_wheel_dir, ) self._wheels += [substra_wheel_dir.relative_to(dest_dir) / wheel_name for wheel_name in substra_wheels] diff --git a/substrafl/exceptions.py b/substrafl/exceptions.py index 294643ea..2737b67f 100644 --- a/substrafl/exceptions.py +++ b/substrafl/exceptions.py @@ -6,10 +6,6 @@ class BatchSizeNotFoundError(Exception): """No batch size found.""" -class SubstraToolsDeprecationWarning(DeprecationWarning): - """The substratools version used is deprecated.""" - - class UnsupportedPythonVersionError(Exception): """The Python version used is not supported by Substra.""" @@ -72,7 +68,7 @@ class InvalidMetricIdentifierError(Exception): class KeyMetadataError(Exception): - """``substrafl_version``, ``substra_version`` and ``substratools_version`` keys can't be added + """``substrafl_version`` and ``substra_version`` keys can't be added to the experiment metadata.""" diff --git a/substrafl/experiment.py b/substrafl/experiment.py index 86b71101..9995905c 100644 --- a/substrafl/experiment.py +++ b/substrafl/experiment.py @@ -14,7 +14,6 @@ from typing import Union import substra -import substratools import substrafl from substrafl.compute_plan_builder import ComputePlanBuilder @@ -183,7 +182,7 @@ def _check_evaluation_strategy( def _check_additional_metadata(additional_metadata: Dict): - unauthorized_keys = {"substrafl_version", "substra_version", "substratools_version", "python_version"} + unauthorized_keys = {"substrafl_version", "substra_version", "python_version"} invalid_keys = set(additional_metadata.keys()).intersection(unauthorized_keys) if len(invalid_keys) > 0: @@ -200,16 +199,15 @@ def _check_additional_metadata(additional_metadata: Dict): def _get_packages_versions() -> dict: - """Returns a dict containing substrafl, substra and substratools versions + """Returns a dict containing substrafl and substra versions Returns: - dict: substrafl, substra and substratools versions + dict: substrafl and substra versions """ return { "substrafl_version": substrafl.__version__, "substra_version": substra.__version__, - "substratools_version": substratools.__version__, "python_version": python_version(), } @@ -452,7 +450,7 @@ def execute_experiment( _check_additional_metadata(additional_metadata) cp_metadata.update(additional_metadata) - # Adding substrafl, substratools and substra versions to the cp metadata + # Adding substrafl and substra versions to the cp metadata cp_metadata.update(_get_packages_versions()) logger.info("Building the compute plan.") diff --git a/substrafl/model_loading.py b/substrafl/model_loading.py index b60d7261..cd415cbf 100644 --- a/substrafl/model_loading.py +++ b/substrafl/model_loading.py @@ -9,7 +9,6 @@ from typing import Optional import substra -import substratools from substra.sdk.models import ComputeTaskStatus import substrafl @@ -21,7 +20,7 @@ logger = logging.getLogger(__name__) -REQUIRED_LIBS = [substrafl, substra, substratools] +REQUIRED_LIBS = [substrafl, substra] REQUIRED_KEYS = set([lib.__name__ + "_version" for lib in REQUIRED_LIBS] + ["python_version"]) METADATA_FILE = "metadata.json" FUNCTION_DICT_KEY = "function_file" diff --git a/substrafl/nodes/utils.py b/substrafl/nodes/utils.py index 815b6c57..9b397908 100644 --- a/substrafl/nodes/utils.py +++ b/substrafl/nodes/utils.py @@ -2,7 +2,6 @@ from typing import List import substra -import substratools def preload_data( @@ -24,9 +23,9 @@ def preload_data( """ dataset_info = client.get_dataset(data_manager_key) - opener_interface = substratools.utils.load_interface_from_module( + opener_interface = substra.tools.utils.load_interface_from_module( "opener", - interface_class=substratools.Opener, + interface_class=substra.tools.Opener, interface_signature=None, path=dataset_info.opener.storage_address, ) diff --git a/substrafl/remote/register/register.py b/substrafl/remote/register/register.py index 55745fb8..cc48df37 100644 --- a/substrafl/remote/register/register.py +++ b/substrafl/remote/register/register.py @@ -21,9 +21,6 @@ logger = logging.getLogger(__name__) -# Substra tools version for which the image naming scheme changed -MINIMAL_DOCKER_SUBSTRATOOLS_VERSION = "0.16.0" - # 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 = 10 # 3.10 @@ -100,7 +97,7 @@ import json import cloudpickle -import substratools as tools +from substra import tools from substrafl.remote.remote_struct import RemoteStruct @@ -182,7 +179,7 @@ def _create_dockerfile(install_libraries: bool, dependencies: Dependency, operat use_gpu=dependencies.use_gpu, custom_binary_dependencies=dependencies.binary_dependencies, ) - # Build Substrafl, Substra and Substratools, and local dependencies wheels if necessary + # Build Substrafl and Substra, and local dependencies wheels if necessary if install_libraries: # generate the copy wheel command copy_wheels_cmd = _generate_copy_local_files(dependencies._wheels) @@ -230,8 +227,7 @@ def _create_substra_function_files( ├── description.md ├── dist │ ├── substra-0.44.0-py3-none-any.whl - │ ├── substrafl-0.36.0-py3-none-any.whl - │ └── substratools-0.20.0-py3-none-any.whl + │ └── substrafl-0.36.0-py3-none-any.whl ├── local_dependencies │ └── local-module-1.6.1-py3-none-any.whl ├── requirements.in diff --git a/substrafl/remote/substratools_methods.py b/substrafl/remote/substratools_methods.py index 5720187a..f56eb267 100644 --- a/substrafl/remote/substratools_methods.py +++ b/substrafl/remote/substratools_methods.py @@ -7,7 +7,7 @@ from typing import TypedDict from typing import Union -import substratools as tools +from substra import tools from substrafl.nodes.schemas import InputIdentifiers from substrafl.nodes.schemas import OutputIdentifiers @@ -158,7 +158,7 @@ def save_instance(self, path: Union[str, os.PathLike]) -> None: self.instance.save_local_state(Path(path)) def register_substratools_function(self): - """Register the function that can be accessed and executed by substratools.""" + """Register the function that can be accessed and executed by substra.""" tools.register( function=self.generic_function, diff --git a/tests/assets_factory.py b/tests/assets_factory.py index 23aedc27..899598c1 100644 --- a/tests/assets_factory.py +++ b/tests/assets_factory.py @@ -1,4 +1,3 @@ -import sys import tempfile from pathlib import Path from typing import List @@ -9,16 +8,10 @@ from substra.sdk.schemas import DatasetSpec from substra.sdk.schemas import Permissions -DEFAULT_SUBSTRATOOLS_VERSION = ( - f"latest-nvidiacuda11.8.0-base-ubuntu22.04-python{sys.version_info.major}.{sys.version_info.minor}" -) - -DEFAULT_SUBSTRATOOLS_DOCKER_IMAGE = f"ghcr.io/substra/substra-tools:{DEFAULT_SUBSTRATOOLS_VERSION}" - DEFAULT_OPENER_FILE = """ import os import numpy as np -import substratools as tools +from substra import tools class NumpyOpener(tools.Opener): def get_data(self, folders): diff --git a/tests/dependency/test_manage_dependencies.py b/tests/dependency/test_manage_dependencies.py index f66f37c7..178c1a5e 100644 --- a/tests/dependency/test_manage_dependencies.py +++ b/tests/dependency/test_manage_dependencies.py @@ -4,7 +4,7 @@ import numpy import pytest -import substratools +import substra from substrafl.dependency.manage_dependencies import build_user_dependency_wheel from substrafl.dependency.manage_dependencies import compile_requirements @@ -36,7 +36,7 @@ def test_build_user_dependency_wheel_invalid_wheel(tmp_path): def test_generate_local_lib_wheel(session_dir): # Test that editable wheel are generated - libs = [substratools] + libs = [substra] dest_dir = Path(tempfile.mkdtemp(dir=session_dir.as_posix())) local_lib_wheels( @@ -56,18 +56,18 @@ def test_generate_local_lib_wheel(session_dir): def test_get_pypi_dependencies_versions(): # Test that pypi wheel can be downloaded - libs = [substratools, numpy] + libs = [substra, numpy] # We check that we have access to the pypi repo not the specific packages version otherwise this test will fail # when trying to create a new version of substrafl as the running dev version on the ci and on a local computer # (0.x.0) won't have been released yet. dependencies = get_pypi_dependencies_versions(lib_modules=libs) - assert dependencies == [f"substratools=={substratools.__version__}", f"numpy=={numpy.__version__}"] + assert dependencies == [f"substra=={substra.__version__}", f"numpy=={numpy.__version__}"] def test_compile_requirements(tmp_path): - compile_requirements(["substrafl", "substra", "substratools", "numpy"], dest_dir=tmp_path) + compile_requirements(["substrafl", "substra", "numpy"], dest_dir=tmp_path) requirements_path = tmp_path / "requirements.txt" assert requirements_path.exists() assert "substrafl" in requirements_path.read_text() diff --git a/tests/remote/register/test_register.py b/tests/remote/register/test_register.py index 8ad17e70..973d57bd 100644 --- a/tests/remote/register/test_register.py +++ b/tests/remote/register/test_register.py @@ -4,7 +4,6 @@ import pytest import substra -import substratools import substrafl from substrafl.dependency import Dependency @@ -76,7 +75,6 @@ def test_create_dockerfile(tmp_path, local_installable_module): python_version = f"{sys.version_info.major}.{sys.version_info.minor}" substrafl_wheel = f"substrafl_internal/dist/substrafl-{substrafl.__version__}-py3-none-any.whl" substra_wheel = f"substrafl_internal/dist/substra-{substra.__version__}-py3-none-any.whl" - substratools_wheel = f"substrafl_internal/dist/substratools-{substratools.__version__}-py3-none-any.whl" local_installable_dependencies = local_installable_module(tmp_path) local_installable_wheel = "substrafl_internal/local_dependencies/mymodule-1.0.2-py3-none-any.whl" local_code_folder = tmp_path / "local" @@ -114,7 +112,6 @@ def test_create_dockerfile(tmp_path, local_installable_module): # Copy local wheels COPY {substrafl_wheel} {substrafl_wheel} COPY {substra_wheel} {substra_wheel} -COPY {substratools_wheel} {substratools_wheel} COPY {local_installable_wheel} {local_installable_wheel} # Copy requirements.txt diff --git a/tests/test_model_loading.py b/tests/test_model_loading.py index 19a348f5..e17c931c 100644 --- a/tests/test_model_loading.py +++ b/tests/test_model_loading.py @@ -11,7 +11,6 @@ import pytest import substra -import substratools import substrafl from substrafl import exceptions @@ -57,7 +56,6 @@ def fake_compute_plan(): compute_plan.metadata = { "substrafl_version": substrafl.__version__, "substra_version": substra.__version__, - "substratools_version": substratools.__version__, "python_version": python_version(), }