From 43d3be9f32eda2ebfe0144c96562f005bd080eaa Mon Sep 17 00:00:00 2001 From: Timothy Willard <9395586+TimothyWillard@users.noreply.github.com> Date: Fri, 8 Nov 2024 12:03:10 -0500 Subject: [PATCH] Rename `flepimop batch` to `submit` Also corrected bugs in unit tests of `JobTimeLimit` with new `BatchSystem` enum. --- flepimop/gempyor_pkg/src/gempyor/batch.py | 8 ++++---- ...__click_batch.py => test__click_submit.py} | 12 +++++------ .../tests/batch/test_job_time_limit_class.py | 20 +++++++++---------- 3 files changed, 20 insertions(+), 20 deletions(-) rename flepimop/gempyor_pkg/tests/batch/{test__click_batch.py => test__click_submit.py} (85%) diff --git a/flepimop/gempyor_pkg/src/gempyor/batch.py b/flepimop/gempyor_pkg/src/gempyor/batch.py index 1abcc02d9..2c12085ad 100644 --- a/flepimop/gempyor_pkg/src/gempyor/batch.py +++ b/flepimop/gempyor_pkg/src/gempyor/batch.py @@ -603,7 +603,7 @@ def _job_name(name: str | None, timestamp: datetime | None) -> str: @cli.command( - name="batch", + name="submit", params=[config_files_argument] + list(config_file_options.values()) + [ @@ -739,7 +739,7 @@ def _job_name(name: str | None, timestamp: datetime | None) -> str: + list(verbosity_options.values()), ) @click.pass_context -def _click_batch(ctx: click.Context = mock_context, **kwargs) -> None: +def _click_submit(ctx: click.Context = mock_context, **kwargs) -> None: """Submit batch jobs""" # Generic setup now = datetime.now(timezone.utc) @@ -754,7 +754,7 @@ def _click_batch(ctx: click.Context = mock_context, **kwargs) -> None: or cfg["inference"]["method"].as_str() != "emcee" ): raise NotImplementedError( - "The `flepimop batch` CLI only supports EMCEE inference jobs." + "The `flepimop submit` CLI only supports EMCEE inference jobs." ) inference_method = cfg["inference"]["method"].as_str() inference_method = ( @@ -776,7 +776,7 @@ def _click_batch(ctx: click.Context = mock_context, **kwargs) -> None: if batch_system != BatchSystem.SLURM: # Temporary limitation raise NotImplementedError( - "The `flepimop batch` CLI only supports batch submission to slurm." + "The `flepimop submit` CLI only supports batch submission to slurm." ) logger.info("Constructing a job to submit to %s", batch_system) if batch_system != BatchSystem.SLURM and kwargs["email"] is not None: diff --git a/flepimop/gempyor_pkg/tests/batch/test__click_batch.py b/flepimop/gempyor_pkg/tests/batch/test__click_submit.py similarity index 85% rename from flepimop/gempyor_pkg/tests/batch/test__click_batch.py rename to flepimop/gempyor_pkg/tests/batch/test__click_submit.py index f0c36dac2..abe4fda61 100644 --- a/flepimop/gempyor_pkg/tests/batch/test__click_batch.py +++ b/flepimop/gempyor_pkg/tests/batch/test__click_submit.py @@ -6,7 +6,7 @@ import pytest import yaml -from gempyor.batch import _click_batch +from gempyor.batch import _click_submit @pytest.fixture @@ -34,13 +34,13 @@ def test_only_inference_emcee_supported_not_implemented_error( yaml.dump(config, f) runner = CliRunner() - result = runner.invoke(_click_batch, [str(config_file.absolute())]) + result = runner.invoke(_click_submit, [str(config_file.absolute())]) assert result.exit_code == 1 assert isinstance(result.exception, NotImplementedError) assert ( str(result.exception) - == "The `flepimop batch` CLI only supports EMCEE inference jobs." + == "The `flepimop submit` CLI only supports EMCEE inference jobs." ) @@ -55,13 +55,13 @@ def test_only_slurm_batch_system_supported_not_implemented_error( yaml.dump({"inference": {"method": "emcee"}}, f) runner = CliRunner() - result = runner.invoke(_click_batch, args + [str(config_file.absolute())]) + result = runner.invoke(_click_submit, args + [str(config_file.absolute())]) assert result.exit_code == 1 assert isinstance(result.exception, NotImplementedError) assert ( str(result.exception) - == "The `flepimop batch` CLI only supports batch submission to slurm." + == "The `flepimop submit` CLI only supports batch submission to slurm." ) @@ -72,7 +72,7 @@ def test_cluster_required_for_slurm_value_error(tmp_path: Path) -> None: runner = CliRunner() result = runner.invoke( - _click_batch, + _click_submit, [ "--slurm", "--simulations", diff --git a/flepimop/gempyor_pkg/tests/batch/test_job_time_limit_class.py b/flepimop/gempyor_pkg/tests/batch/test_job_time_limit_class.py index f51be9ef7..fddfb7ec3 100644 --- a/flepimop/gempyor_pkg/tests/batch/test_job_time_limit_class.py +++ b/flepimop/gempyor_pkg/tests/batch/test_job_time_limit_class.py @@ -4,7 +4,7 @@ import pytest -from gempyor.batch import JobSize, JobTimeLimit +from gempyor.batch import BatchSystem, JobSize, JobTimeLimit NONPOSITIVE_TIMEDELTAS = (timedelta(), timedelta(hours=-1.0), timedelta(days=-3.0)) @@ -31,14 +31,16 @@ def test_time_limit_non_positive_value_error(time_limit: timedelta) -> None: timedelta(minutes=12345), ), ) -@pytest.mark.parametrize("batch_system", (None, "aws", "local", "slurm")) +@pytest.mark.parametrize( + "batch_system", (None, BatchSystem.AWS, BatchSystem.LOCAL, BatchSystem.SLURM) +) def test_format_output_validation( - time_limit: timedelta, batch_system: Literal["aws", "local", "slurm"] | None + time_limit: timedelta, batch_system: BatchSystem | None ) -> None: job_time_limit = JobTimeLimit(time_limit=time_limit) formatted_time_limit = job_time_limit.format(batch_system=batch_system) assert isinstance(formatted_time_limit, str) - if batch_system == "slurm": + if batch_system == BatchSystem.SLURM: assert re.match(r"^[0-9]+\:[0-9]{2}\:[0-9]{2}$", formatted_time_limit) else: assert formatted_time_limit.isdigit() @@ -50,15 +52,13 @@ def test_format_output_validation( (timedelta(hours=1), None, "60"), (timedelta(seconds=20), None, "1"), (timedelta(days=2, hours=3, minutes=45), None, "3105"), - (timedelta(hours=1), "slurm", "1:00:00"), - (timedelta(seconds=20), "slurm", "0:00:20"), - (timedelta(days=1, hours=2, minutes=34, seconds=5), "slurm", "26:34:05"), + (timedelta(hours=1), BatchSystem.SLURM, "1:00:00"), + (timedelta(seconds=20), BatchSystem.SLURM, "0:00:20"), + (timedelta(days=1, hours=2, minutes=34, seconds=5), BatchSystem.SLURM, "26:34:05"), ), ) def test_format_exact_results( - time_limit: timedelta, - batch_system: Literal["aws", "local", "slurm"] | None, - expected: str, + time_limit: timedelta, batch_system: BatchSystem | None, expected: str ) -> None: job_time_limit = JobTimeLimit(time_limit=time_limit) assert job_time_limit.format(batch_system=batch_system) == expected