Skip to content

infra: use fixture for Python version in scikit-learn tests #1630

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

Merged
merged 2 commits into from
Jun 25, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 7 additions & 3 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ def pytest_addoption(parser):
parser.addoption(
"--rl-ray-full-version", action="store", default=RLEstimator.RAY_LATEST_VERSION
)
parser.addoption("--sklearn-full-version", action="store", default="0.20.0")
parser.addoption("--ei-tf-full-version", action="store")
parser.addoption("--xgboost-full-version", action="store", default="1.0-1")

Expand Down Expand Up @@ -299,8 +298,13 @@ def rl_ray_full_version(request):


@pytest.fixture(scope="module")
def sklearn_full_version(request):
return request.config.getoption("--sklearn-full-version")
def sklearn_full_version():
return "0.20.0"


@pytest.fixture(scope="module")
def sklearn_full_py_version():
return "py3"


@pytest.fixture(scope="module")
Expand Down
4 changes: 2 additions & 2 deletions tests/integ/test_airflow_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ def test_mxnet_airflow_config_uploads_data_source_to_s3(

@pytest.mark.canary_quick
def test_sklearn_airflow_config_uploads_data_source_to_s3(
sagemaker_session, cpu_instance_type, sklearn_full_version
sagemaker_session, cpu_instance_type, sklearn_full_version, sklearn_full_py_version
):
with timeout(seconds=AIRFLOW_CONFIG_TIMEOUT_IN_SECONDS):
script_path = os.path.join(DATA_DIR, "sklearn_mnist", "mnist.py")
Expand All @@ -486,7 +486,7 @@ def test_sklearn_airflow_config_uploads_data_source_to_s3(
role=ROLE,
train_instance_type=cpu_instance_type,
framework_version=sklearn_full_version,
py_version=PYTHON_VERSION,
py_version=sklearn_full_py_version,
sagemaker_session=sagemaker_session,
hyperparameters={"epochs": 1},
)
Expand Down
12 changes: 8 additions & 4 deletions tests/integ/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,9 @@ def test_private_github(sagemaker_local_session, mxnet_full_version, mxnet_full_

@pytest.mark.local_mode
@pytest.mark.skip("needs a secure authentication approach")
def test_private_github_with_2fa(sagemaker_local_session, sklearn_full_version):
def test_private_github_with_2fa(
sagemaker_local_session, sklearn_full_version, sklearn_full_py_version
):
script_path = "mnist.py"
data_path = os.path.join(DATA_DIR, "sklearn_mnist")
git_config = {
Expand All @@ -149,7 +151,7 @@ def test_private_github_with_2fa(sagemaker_local_session, sklearn_full_version):
entry_point=script_path,
role="SageMakerRole",
source_dir=source_dir,
py_version="py3", # Scikit-learn supports only Python 3
py_version=sklearn_full_py_version,
train_instance_count=1,
train_instance_type="local",
sagemaker_session=sagemaker_local_session,
Expand Down Expand Up @@ -187,7 +189,9 @@ def test_private_github_with_2fa(sagemaker_local_session, sklearn_full_version):


@pytest.mark.local_mode
def test_github_with_ssh_passphrase_not_configured(sagemaker_local_session, sklearn_full_version):
def test_github_with_ssh_passphrase_not_configured(
sagemaker_local_session, sklearn_full_version, sklearn_full_py_version
):
script_path = "mnist.py"
data_path = os.path.join(DATA_DIR, "sklearn_mnist")
git_config = {
Expand All @@ -201,11 +205,11 @@ def test_github_with_ssh_passphrase_not_configured(sagemaker_local_session, skle
entry_point=script_path,
role="SageMakerRole",
source_dir=source_dir,
py_version="py3", # Scikit-learn supports only Python 3
train_instance_count=1,
train_instance_type="local",
sagemaker_session=sagemaker_local_session,
framework_version=sklearn_full_version,
py_version=sklearn_full_py_version,
hyperparameters={"epochs": 1},
git_config=git_config,
)
Expand Down
48 changes: 28 additions & 20 deletions tests/integ/test_sklearn_train.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from sagemaker.sklearn import SKLearn
from sagemaker.sklearn import SKLearnModel
from sagemaker.utils import sagemaker_timestamp, unique_name_from_base
from tests.integ import DATA_DIR, PYTHON_VERSION, TRAINING_DEFAULT_TIMEOUT_MINUTES
from tests.integ import DATA_DIR, TRAINING_DEFAULT_TIMEOUT_MINUTES
from tests.integ.timeout import timeout, timeout_and_delete_endpoint_by_name


Expand All @@ -30,14 +30,17 @@
reason="This test has always failed, but the failure was masked by a bug. "
"This test should be fixed. Details in https://github.com/aws/sagemaker-python-sdk/pull/968"
)
def sklearn_training_job(sagemaker_session, sklearn_full_version, cpu_instance_type):
return _run_mnist_training_job(sagemaker_session, cpu_instance_type, sklearn_full_version)
def sklearn_training_job(
sagemaker_session, sklearn_full_version, sklearn_full_py_version, cpu_instance_type
):
return _run_mnist_training_job(
sagemaker_session, cpu_instance_type, sklearn_full_version, sklearn_full_py_version
)
sagemaker_session.boto_region_name


@pytest.mark.skipif(PYTHON_VERSION != "py3", reason="Scikit-learn image supports only Python 3.")
def test_training_with_additional_hyperparameters(
sagemaker_session, sklearn_full_version, cpu_instance_type
sagemaker_session, sklearn_full_version, sklearn_full_py_version, cpu_instance_type
):
with timeout(minutes=TRAINING_DEFAULT_TIMEOUT_MINUTES):
script_path = os.path.join(DATA_DIR, "sklearn_mnist", "mnist.py")
Expand All @@ -48,7 +51,7 @@ def test_training_with_additional_hyperparameters(
role="SageMakerRole",
train_instance_type=cpu_instance_type,
framework_version=sklearn_full_version,
py_version=PYTHON_VERSION,
py_version=sklearn_full_py_version,
sagemaker_session=sagemaker_session,
hyperparameters={"epochs": 1},
)
Expand All @@ -65,9 +68,8 @@ def test_training_with_additional_hyperparameters(
return sklearn.latest_training_job.name


@pytest.mark.skipif(PYTHON_VERSION != "py3", reason="Scikit-learn image supports only Python 3.")
def test_training_with_network_isolation(
sagemaker_session, sklearn_full_version, cpu_instance_type
sagemaker_session, sklearn_full_version, sklearn_full_py_version, cpu_instance_type
):
with timeout(minutes=TRAINING_DEFAULT_TIMEOUT_MINUTES):
script_path = os.path.join(DATA_DIR, "sklearn_mnist", "mnist.py")
Expand All @@ -78,7 +80,7 @@ def test_training_with_network_isolation(
role="SageMakerRole",
train_instance_type=cpu_instance_type,
framework_version=sklearn_full_version,
py_version=PYTHON_VERSION,
py_version=sklearn_full_py_version,
sagemaker_session=sagemaker_session,
hyperparameters={"epochs": 1},
enable_network_isolation=True,
Expand All @@ -101,7 +103,6 @@ def test_training_with_network_isolation(

@pytest.mark.canary_quick
@pytest.mark.regional_testing
@pytest.mark.skipif(PYTHON_VERSION != "py3", reason="Scikit-learn image supports only python 3.")
@pytest.mark.skip(
reason="This test has always failed, but the failure was masked by a bug. "
"This test should be fixed. Details in https://github.com/aws/sagemaker-python-sdk/pull/968"
Expand All @@ -115,13 +116,16 @@ def test_attach_deploy(sklearn_training_job, sagemaker_session, cpu_instance_typ
_predict_and_assert(predictor)


@pytest.mark.skipif(PYTHON_VERSION != "py3", reason="Scikit-learn image supports only python 3.")
@pytest.mark.skip(
reason="This test has always failed, but the failure was masked by a bug. "
"This test should be fixed. Details in https://github.com/aws/sagemaker-python-sdk/pull/968"
)
def test_deploy_model(
sklearn_training_job, sagemaker_session, cpu_instance_type, sklearn_full_version
sklearn_training_job,
sagemaker_session,
cpu_instance_type,
sklearn_full_version,
sklearn_full_py_version,
):
endpoint_name = "test-sklearn-deploy-model-{}".format(sagemaker_timestamp())
with timeout_and_delete_endpoint_by_name(endpoint_name, sagemaker_session):
Expand All @@ -141,12 +145,13 @@ def test_deploy_model(
_predict_and_assert(predictor)


@pytest.mark.skipif(PYTHON_VERSION != "py3", reason="Scikit-learn image supports only python 3.")
@pytest.mark.skip(
reason="This test has always failed, but the failure was masked by a bug. "
"This test should be fixed. Details in https://github.com/aws/sagemaker-python-sdk/pull/968"
)
def test_async_fit(sagemaker_session, cpu_instance_type, sklearn_full_version):
def test_async_fit(
sagemaker_session, cpu_instance_type, sklearn_full_version, sklearn_full_py_version
):
endpoint_name = "test-sklearn-attach-deploy-{}".format(sagemaker_timestamp())

with timeout(minutes=5):
Expand All @@ -169,8 +174,9 @@ def test_async_fit(sagemaker_session, cpu_instance_type, sklearn_full_version):
_predict_and_assert(predictor)


@pytest.mark.skipif(PYTHON_VERSION != "py3", reason="Scikit-learn image supports only python 3.")
def test_failed_training_job(sagemaker_session, sklearn_full_version, cpu_instance_type):
def test_failed_training_job(
sagemaker_session, sklearn_full_version, sklearn_full_py_version, cpu_instance_type
):
with timeout(minutes=TRAINING_DEFAULT_TIMEOUT_MINUTES):
script_path = os.path.join(DATA_DIR, "sklearn_mnist", "failure_script.py")
data_path = os.path.join(DATA_DIR, "sklearn_mnist")
Expand All @@ -179,7 +185,7 @@ def test_failed_training_job(sagemaker_session, sklearn_full_version, cpu_instan
entry_point=script_path,
role="SageMakerRole",
framework_version=sklearn_full_version,
py_version=PYTHON_VERSION,
py_version=sklearn_full_py_version,
train_instance_count=1,
train_instance_type=cpu_instance_type,
sagemaker_session=sagemaker_session,
Expand All @@ -194,7 +200,9 @@ def test_failed_training_job(sagemaker_session, sklearn_full_version, cpu_instan
sklearn.fit(train_input, job_name=job_name)


def _run_mnist_training_job(sagemaker_session, instance_type, sklearn_full_version, wait=True):
def _run_mnist_training_job(
sagemaker_session, instance_type, sklearn_version, py_version, wait=True
):
with timeout(minutes=TRAINING_DEFAULT_TIMEOUT_MINUTES):

script_path = os.path.join(DATA_DIR, "sklearn_mnist", "mnist.py")
Expand All @@ -204,8 +212,8 @@ def _run_mnist_training_job(sagemaker_session, instance_type, sklearn_full_versi
sklearn = SKLearn(
entry_point=script_path,
role="SageMakerRole",
framework_version=sklearn_full_version,
py_version=PYTHON_VERSION,
framework_version=sklearn_version,
py_version=py_version,
train_instance_type=instance_type,
sagemaker_session=sagemaker_session,
hyperparameters={"epochs": 1},
Expand Down