From ab4e7c915c736880c0cca65b2b1cd1496348110f Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Mon, 8 Apr 2024 16:29:47 +0100 Subject: [PATCH 1/8] Add expected newlines into test_output Out test cases were falling over when cats printed newlines which were unexpected in the test cases. This puts the newlines in the expected strings in the tests. --- tests/test_output.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_output.py b/tests/test_output.py index 2f16e19..74bae78 100644 --- a/tests/test_output.py +++ b/tests/test_output.py @@ -31,10 +31,11 @@ @pytest.mark.parametrize( "output,expected", [ - (OUTPUT, "Best job start time: 2024-03-16 02:00:00"), + (OUTPUT, "Best job start time: 2024-03-16 02:00:00\n"), ( OUTPUT_WITH_EMISSION_ESTIMATE, """Best job start time: 2024-03-16 02:00:00 + Estimated emmissions for running job now: 19 Estimated emmissions for running delayed job: 9 (- 10)""", ), From 356f10bfd5a1f05a24a39e2b799eba8c9bda1280 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Tue, 9 Apr 2024 09:15:15 +0100 Subject: [PATCH 2/8] Enable mypy to check out test hints --- .github/workflows/tests.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9c33f98..065f04a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -26,7 +26,7 @@ jobs: steps: - uses: actions/checkout@v3 - - name: Set up Python 3.10 + - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} @@ -39,6 +39,11 @@ jobs: flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics + - name: Check types with mypy + run: | + python3 -m pip install mypy + python3 -m pip install types-PyYAML types-redis types-requests types-ujson + python3 -m mypy cats - name: Test with pytest run: | python3 -m pytest From d85710bd73a354ac1cc837a79ad4e4bacaef87a2 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Tue, 9 Apr 2024 09:26:44 +0100 Subject: [PATCH 3/8] Fix typo in logging eror -> error, mypy picks this up --- cats/configure.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cats/configure.py b/cats/configure.py index f7b73f7..8aa5f2e 100644 --- a/cats/configure.py +++ b/cats/configure.py @@ -47,7 +47,7 @@ def get_runtime_config(args) -> tuple[dict, APIInterface, str, int]: try: duration = int(args.duration) except ValueError: - logging.eror(msg) + logging.error(msg) raise ValueError if duration <= 0: logging.error(msg) @@ -91,13 +91,14 @@ def CI_API_from_config_or_args(args, config) -> APIInterface: api = "carbonintensity.org.uk" # default value logging.warning(f"Unspecified carbon intensity forecast service, using {api}") try: - return API_interfaces[api] + interface = API_interfaces[api] except KeyError: logging.error( f"Error: {api} is not a valid API choice. It must be one of " "\n".join( API_interfaces.keys() ) ) + return interface def get_location_from_config_or_args(args, config) -> str: From 199955fa477a44bb7c3ef23a878ca8bf794bbf62 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Tue, 9 Apr 2024 09:37:01 +0100 Subject: [PATCH 4/8] Fix mypy issues in __init__ Always return an integer from main (so we sys.exit 0 if everything works). Don't reuse output for the subrocess output (bytes) and the cats output type that goes into the subprocess. --- cats/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cats/__init__.py b/cats/__init__.py index 3318da4..994b1e6 100644 --- a/cats/__init__.py +++ b/cats/__init__.py @@ -196,7 +196,7 @@ def to_json(self, dateformat: str = "", **kwargs) -> str: def schedule_at(output: CATSOutput, args: list[str]) -> None: "Schedule job with optimal start time using at(1)" proc = subprocess.Popen(args, stdout=subprocess.PIPE) - output = subprocess.check_output( + proc_output = subprocess.check_output( ( "at", "-t", @@ -206,7 +206,7 @@ def schedule_at(output: CATSOutput, args: list[str]) -> None: ) -def main(arguments=None) -> Optional[int]: +def main(arguments=None) -> int: parser = parse_arguments() args = parser.parse_args(arguments) if args.command and not args.scheduler: @@ -272,6 +272,7 @@ def main(arguments=None) -> Optional[int]: print(output) if args.command and args.scheduler == "at": schedule_at(output, args.command.split()) + return 0 if __name__ == "__main__": From 058a780daed5dbb2b8696921c9308ae98aecf85d Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Tue, 9 Apr 2024 11:02:46 +0100 Subject: [PATCH 5/8] Fix return type and caller of get_runtime_config The return type now matches what we return, but could probably use some cleanup. We don't make use of the returned _jobinfo and _PUE but we probably should. We pull out jobinfo below in main. We probably shouldn't. --- cats/__init__.py | 2 +- cats/configure.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cats/__init__.py b/cats/__init__.py index 994b1e6..5aaf9a0 100644 --- a/cats/__init__.py +++ b/cats/__init__.py @@ -215,7 +215,7 @@ def main(arguments=None) -> int: " specify the scheduler with the -s or --scheduler option" ) return 1 - config, CI_API_interface, location, duration = get_runtime_config(args) + config, CI_API_interface, location, duration, _jobinfo, _PUE = get_runtime_config(args) ######################## ## Obtain CI forecast ## diff --git a/cats/configure.py b/cats/configure.py index 8aa5f2e..305a288 100644 --- a/cats/configure.py +++ b/cats/configure.py @@ -13,7 +13,7 @@ import logging import sys from collections.abc import Mapping -from typing import Any +from typing import Any, Union import requests import yaml @@ -24,7 +24,8 @@ __all__ = ["get_runtime_config"] -def get_runtime_config(args) -> tuple[dict, APIInterface, str, int]: +def get_runtime_config(args) -> tuple[Mapping[str, Any], APIInterface, str, int, + Union[list[tuple[int, float]],None],Any]: """Return the runtime cats configuration from list of command line arguments and content of configuration file. From b930f419d714715be298869b447bb494f41843f2 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Tue, 9 Apr 2024 11:19:50 +0100 Subject: [PATCH 6/8] Sort out typing problems with check_clean_arguments But I cannot help but think that this is a long way around. Unwrapping the dictonary construtor so that mypy can see the types (aroud line 32) seems more complex than it should be, and the cast at the end seems painful. Can we not just build the TypedDict as we go? --- cats/check_clean_arguments.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/cats/check_clean_arguments.py b/cats/check_clean_arguments.py index a301f06..3af20eb 100644 --- a/cats/check_clean_arguments.py +++ b/cats/check_clean_arguments.py @@ -1,6 +1,6 @@ import re import sys -from typing import Iterable, Optional, TypedDict +from typing import Iterable, Optional, TypedDict, cast class JobInfo(TypedDict): @@ -28,7 +28,11 @@ def validate_jobinfo( "cpus", "gpus", ) - info = dict([match.groups() for match in re.finditer(r"(\w+)=([\w.]+)", jobinfo)]) + + info_list = [match.groups() for match in re.finditer(r"(\w+)=([\w.]+)", jobinfo)] + info = {} + for item in info_list: + info[item[0]] = item[1] # Check if some information is missing if missing_keys := set(expected_info_keys) - set(info.keys()): @@ -46,11 +50,11 @@ def validate_jobinfo( for key in [k for k in info if k != "partition"]: try: info[key] = int(info[key]) - assert info[key] >= 0 + assert int(info[key]) >= 0 except (ValueError, AssertionError): sys.stderr.write( f"ERROR: job info key {key} should be a positive integer\n" ) return None - return JobInfo(info) + return cast(JobInfo, info) From a257b9fbbce27f518bc287222fd5336e790a147f Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Tue, 9 Apr 2024 17:01:26 +0100 Subject: [PATCH 7/8] Put type testing reqs into pyptoject.toml --- .github/workflows/tests.yml | 3 +-- pyproject.toml | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 065f04a..726c03c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -41,8 +41,7 @@ jobs: flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics - name: Check types with mypy run: | - python3 -m pip install mypy - python3 -m pip install types-PyYAML types-redis types-requests types-ujson + python3 -m pip install '.[types]' python3 -m mypy cats - name: Test with pytest run: | diff --git a/pyproject.toml b/pyproject.toml index 133461d..f796535 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,6 +18,7 @@ [project.optional-dependencies] test = ["pytest", "numpy>=1.5.0", "pytest-subprocess==1.5.0"] + types = ["mypy", "types-PyYAML", "types-redis", "types-requests", "types-ujson"] [project.urls] Home = "https://github.com/GreenScheduler/cats" From 5d30fb72caa406233997881759ea8107437f12d6 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Wed, 10 Apr 2024 09:09:12 +0100 Subject: [PATCH 8/8] Have tests report coverage in CI This will help us keep track of current test coverage --- .github/workflows/tests.yml | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 726c03c..44442f5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -45,4 +45,4 @@ jobs: python3 -m mypy cats - name: Test with pytest run: | - python3 -m pytest + python3 -m pytest --cov diff --git a/pyproject.toml b/pyproject.toml index f796535..f7b763e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,7 +17,7 @@ dependencies = ["requests-cache>=1.0", "PyYAML>=6.0"] [project.optional-dependencies] - test = ["pytest", "numpy>=1.5.0", "pytest-subprocess==1.5.0"] + test = ["pytest", "numpy>=1.5.0", "pytest-subprocess==1.5.0", "pytest-cov"] types = ["mypy", "types-PyYAML", "types-redis", "types-requests", "types-ujson"] [project.urls]