From 40c24196946c486a7cbd576e2fd35c3052721821 Mon Sep 17 00:00:00 2001 From: Christian Krudewig Date: Wed, 9 Oct 2024 00:13:30 +0200 Subject: [PATCH] Moving linting to ruff (#121) * feat: Add ruff configuration * Deactivate flake8 in precommit * chore: Fixing ruff warnings * ci: Minimal ruff workflow * ci: Replace isort and black by ruff * ci: Disable checks for python 3.7 and 3.8 which are end-of-life * doc: Fix dead link * ci: fix codecov upload --- .github/workflows/ruff.yml | 18 ++++++++++ .github/workflows/test.yml | 7 ++-- .pre-commit-config.yaml | 20 ++++-------- dike/__init__.py | 3 +- dike/_batch.py | 38 +++++++++++++--------- dike/_limit_jobs.py | 9 ++--- dike/_retry.py | 13 ++++---- docs/contributing.md | 4 --- examples/ml_prediction_service/api.py | 22 ++++++++----- ruff.toml | 47 +++++++++++++++++++++++++++ tasks.py | 4 +-- tests/test_batch.py | 25 +++++++++----- tests/test_limit_jobs.py | 19 ++++++----- tests/test_wrap_in_coroutine.py | 19 ++++++----- tox.ini | 21 ++---------- 15 files changed, 165 insertions(+), 104 deletions(-) create mode 100644 .github/workflows/ruff.yml create mode 100644 ruff.toml diff --git a/.github/workflows/ruff.yml b/.github/workflows/ruff.yml new file mode 100644 index 0000000..49da7c6 --- /dev/null +++ b/.github/workflows/ruff.yml @@ -0,0 +1,18 @@ +name: Ruff +on: push +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install Python + uses: actions/setup-python@v5 + with: + python-version: "3.11" + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install ruff + # Update output format to enable automatic inline annotations. + - name: Run Ruff + run: ruff check --output-format=github . diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7901e81..f173896 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -22,7 +22,7 @@ jobs: strategy: matrix: os: [ubuntu-latest, macos-latest, windows-latest] - python-version: ["3.7", "3.8", "3.9", "3.10"] + python-version: ["3.9", "3.10"] exclude: # No numpy wheels yet: - os: windows-latest @@ -62,7 +62,7 @@ jobs: pip install poetry tox tox-gh-actions poetry install --with test,dev,doc - - name: Lint and test with tox + - name: Test with tox if: ${{ matrix.tox-full-run }} run: poetry run tox @@ -77,10 +77,11 @@ jobs: with: junit_files: pytest.xml - - uses: codecov/codecov-action@v3 + - uses: codecov/codecov-action@v4 if: ${{ matrix.publish-results && always() }} with: fail_ci_if_error: true + token: ${{ secrets.CODECOV_TOKEN }} files: coverage.xml publish_dev_build: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 15154b4..bcce623 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,7 +5,7 @@ ci: submodules: false repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.4.0 + rev: v5.0.0 hooks: - id: check-merge-conflict - id: check-json @@ -14,17 +14,9 @@ repos: args: [--unsafe] - id: debug-statements - id: end-of-file-fixer -- repo: https://github.com/pre-commit/mirrors-isort - rev: v5.10.1 +- repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.6.9 hooks: - - id: isort -- repo: https://github.com/ambv/black - rev: 22.10.0 - hooks: - - id: black - language_version: python3.9 -- repo: https://github.com/pycqa/flake8 - rev: 6.0.0 - hooks: - - id: flake8 - additional_dependencies: [flake8-typing-imports==1.14.0] + - id: ruff + args: [ --fix ] + - id: ruff-format diff --git a/dike/__init__.py b/dike/__init__.py index 25fce49..787ee84 100644 --- a/dike/__init__.py +++ b/dike/__init__.py @@ -1,4 +1,5 @@ -"""Decorator library""" +"""Decorator library.""" + import functools import inspect from typing import Callable diff --git a/dike/_batch.py b/dike/_batch.py index 76f0f09..b034142 100644 --- a/dike/_batch.py +++ b/dike/_batch.py @@ -1,4 +1,5 @@ -"""Implementation of the @dike.batch decorator""" +"""Implementation of the @dike.batch decorator.""" + import asyncio import functools from contextlib import contextmanager @@ -11,8 +12,7 @@ # Deactivate mccabe's complexity warnings which doesn't like closures -# flake8: noqa: C901 -def batch( +def batch( # noqa: PLR0915, C901 *, target_batch_size: int, max_waiting_time: float, @@ -20,9 +20,10 @@ def batch( argument_type: str = "list", ) -> Callable[[Callable[..., Coroutine[Any, Any, Any]]], Callable[..., Coroutine[Any, Any, Any]]]: """@batch is a decorator to cumulate function calls and process them in batches. - Not thread-safe. - The function to wrap must have arguments of type list or numpy.array which can be aggregated. + Not thread-safe. + + The function to wrap must have arguments of type list or `numpy.array` which can be aggregated. It must return just a single value of the same type. The type has to be specified with the `argument_type` parameter of the decorator. @@ -102,7 +103,7 @@ def batch( if argument_type == "numpy" and np is None: raise ValueError('Unable to use "numpy" as argument_type because numpy is not available') - def decorator(func): + def decorator(func): # noqa: C901, PLR0915 next_free_batch: int = 0 call_args_queue: List[Tuple[List, Dict]] = [] n_rows_in_queue: int = 0 @@ -112,8 +113,13 @@ def decorator(func): @functools.wraps(func) async def batching_call(*args, **kwargs): - """This is the actual wrapper function which controls the process""" - nonlocal results, num_results_ready, result_ready_events, call_args_queue, n_rows_in_queue + """This is the actual wrapper function which controls the process.""" + nonlocal \ + results, \ + num_results_ready, \ + result_ready_events, \ + call_args_queue, \ + n_rows_in_queue with enqueue(args, kwargs) as (my_batch_no, start_index, stop_index): await wait_for_calculation(my_batch_no) @@ -121,7 +127,7 @@ async def batching_call(*args, **kwargs): @contextmanager def enqueue(args, kwargs) -> (int, int, int): - """Add call arguments to queue and get the batch number and result indices""" + """Add call arguments to queue and get the batch number and result indices.""" batch_no = next_free_batch if batch_no not in result_ready_events: result_ready_events[batch_no] = asyncio.Event() @@ -132,7 +138,7 @@ def enqueue(args, kwargs) -> (int, int, int): remove_result(batch_no) def add_args_to_queue(args, kwargs): - """Add a new argument vector to the queue and return result indices""" + """Add a new argument vector to the queue and return result indices.""" nonlocal call_args_queue, n_rows_in_queue if call_args_queue and ( @@ -155,7 +161,7 @@ def add_args_to_queue(args, kwargs): return offset, n_rows_in_queue async def wait_for_calculation(batch_no_to_calculate): - """Pause until the result becomes available or trigger the calculation on timeout""" + """Pause until the result becomes available or trigger the calculation on timeout.""" if n_rows_in_queue >= target_batch_size: await calculate(batch_no_to_calculate) else: @@ -173,20 +179,20 @@ async def wait_for_calculation(batch_no_to_calculate): ) async def calculate(batch_no_to_calculate): - """Call the decorated coroutine with batched arguments""" + """Call the decorated coroutine with batched arguments.""" nonlocal results, call_args_queue, num_results_ready if next_free_batch == batch_no_to_calculate: n_results = len(call_args_queue) args, kwargs = pop_args_from_queue() try: results[batch_no_to_calculate] = await func(*args, **kwargs) - except Exception as e: # pylint: disable=broad-except + except Exception as e: # pylint: disable=broad-except # noqa: BLE001 results[batch_no_to_calculate] = e num_results_ready[batch_no_to_calculate] = n_results result_ready_events[batch_no_to_calculate].set() def pop_args_from_queue(): - """Get all collected arguments from the queue as batch""" + """Get all collected arguments from the queue as batch.""" nonlocal next_free_batch, call_args_queue, n_rows_in_queue n_args = len(call_args_queue[0][0]) @@ -215,7 +221,7 @@ def pop_args_from_queue(): return args, kwargs def get_results(start_index: int, stop_index: int, batch_no): - """Pop the results for a certain index range from the output buffer""" + """Pop the results for a certain index range from the output buffer.""" nonlocal results if isinstance(results[batch_no], Exception): @@ -225,7 +231,7 @@ def get_results(start_index: int, stop_index: int, batch_no): return results_to_return def remove_result(batch_no): - """Reduce reference count to output buffer and eventually delete it""" + """Reduce reference count to output buffer and eventually delete it.""" nonlocal num_results_ready, result_ready_events, results if num_results_ready[batch_no] == 1: diff --git a/dike/_limit_jobs.py b/dike/_limit_jobs.py index 8e72204..c9ca923 100644 --- a/dike/_limit_jobs.py +++ b/dike/_limit_jobs.py @@ -1,11 +1,12 @@ -"""Implementation of the @dike.limit_jobs decorator""" +"""Implementation of the @dike.limit_jobs decorator.""" + import functools import inspect from typing import Any, Callable, Coroutine -class TooManyCalls(Exception): - """Error raised by @limit_jobs when a call exceeds the preset limit""" +class TooManyCalls(Exception): # noqa: N818 + """Error raised by @limit_jobs when a call exceeds the preset limit.""" def limit_jobs(*, limit: int) -> Callable[..., Coroutine[Any, Any, Any]]: @@ -63,7 +64,7 @@ def limit_jobs(*, limit: int) -> Callable[..., Coroutine[Any, Any, Any]]: def decorator(func): if not inspect.iscoroutinefunction(func): - raise ValueError(f"Error when wrapping {str(func)}. Only coroutines can be wrapped!") + raise ValueError(f"Error when wrapping {func!s}. Only coroutines can be wrapped!") @functools.wraps(func) async def limited_call(*args, **kwargs): diff --git a/dike/_retry.py b/dike/_retry.py index aca8cfb..6591b55 100644 --- a/dike/_retry.py +++ b/dike/_retry.py @@ -1,19 +1,20 @@ -"""Implementation of the @dike.retry decorator""" +"""Implementation of the @dike.retry decorator.""" + import asyncio import datetime import functools import inspect import logging -from typing import Awaitable, Callable, Tuple, Type, Union +from typing import Awaitable, Callable, Optional, Tuple, Type, Union logger = logging.getLogger("dike") def retry( *, - attempts: int = None, + attempts: Optional[int] = None, exception_types: Union[Type[BaseException], Tuple[Type[BaseException]]] = Exception, - delay: datetime.timedelta = None, + delay: Optional[datetime.timedelta] = None, backoff: int = 1, log_exception_info: bool = True, ) -> Callable[[Callable[..., Awaitable]], Callable[..., Awaitable]]: @@ -70,7 +71,7 @@ def retry( def decorator(func: Callable[..., Awaitable]) -> Callable[..., Awaitable]: if not inspect.iscoroutinefunction(func): - raise ValueError(f"Error when wrapping {str(func)}. Only coroutines can be wrapped!") + raise ValueError(f"Error when wrapping {func!s}. Only coroutines can be wrapped!") @functools.wraps(func) async def guarded_call(*args, **kwargs): @@ -86,7 +87,7 @@ async def guarded_call(*args, **kwargs): if not counter: raise logger.warning( - f"Caught exception {repr(e)}. Retrying in {next_delay:.3g}s ...", + f"Caught exception {e!r}. Retrying in {next_delay:.3g}s ...", exc_info=log_exception_info, ) await asyncio.sleep(next_delay) diff --git a/docs/contributing.md b/docs/contributing.md index 342bc7b..f9a79cb 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -1,7 +1,3 @@ {% include-markdown "../CONTRIBUTING.md" %} - -{% - include-markdown "../AUTHORS.md" -%} diff --git a/examples/ml_prediction_service/api.py b/examples/ml_prediction_service/api.py index 92fc47c..0b76a62 100644 --- a/examples/ml_prediction_service/api.py +++ b/examples/ml_prediction_service/api.py @@ -1,9 +1,13 @@ -# Dependencies: fastapi, uvicorn -# Run with: python api.py -# -# Simple load test with github.com/codesenberg/bombardier: -# bombardier -c 25 -r 300 -d 10s -l 'localhost:8000/predict?number=5' -# +"""Minimal example API using dike for microbatching. + +Dependencies: fastapi, uvicorn +Run with: python api.py + +Simple load test with github.com/codesenberg/bombardier: + + bombardier -c 25 -r 300 -d 10s -l 'localhost:8000/predict?number=5' +""" + import asyncio import concurrent import random @@ -18,7 +22,7 @@ def predict(numbers: List[float]): - """Dummy machine learning operation""" + """Dummy machine learning operation.""" arr = np.array(numbers) for _ in range(10000): arr = np.sqrt(arr + random.random() * 2) @@ -28,17 +32,19 @@ def predict(numbers: List[float]): @dike.limit_jobs(limit=20) @dike.batch(target_batch_size=10, max_waiting_time=0.1) async def predict_in_pool(numbers: List[float]): + """Wrapper function for the predictions to allow batching.""" loop = asyncio.get_running_loop() return await loop.run_in_executor(app.state.pool, predict, numbers) @app.on_event("startup") -async def on_startup(): +async def on_startup(): # noqa: D103 app.state.pool = concurrent.futures.ProcessPoolExecutor(max_workers=2) @app.get("/predict") async def get_predict(number: float, response: Response): + """API endpoint for machine learning inference.""" try: x = await predict_in_pool([number]) return {"result": x} diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 0000000..030e90d --- /dev/null +++ b/ruff.toml @@ -0,0 +1,47 @@ +line-length = 100 +##extend-exclude = ["*.ipynb", "**/*_pb2.py"] + +[lint] +ignore = [ + "PLR0913", # Too many arguments + "S311", # suspicious-non-cryptographic-random-usage + "G004" # Logging statement uses f-string +] +select = [ + "A", # flake8-builtins + "C4", # flake8-comprehensions + "DTZ", # flake8-datetimez + "B", + "B9", + "C", + "G", # flake8-logging-format + "PYI", # flake8-pyi + "PT", # flake8-pytest-style + "TCH", # flake8-type-checking + "C90", # MCCabe + "D", # Pydocstyle + "E", # Pycodestyle error + "F", # Pyflakes + "I", # Isort + "N", # pep8-naming + "W", # Pycodestyle warning +# "ANN", # flake8-annotations + "S", # flake8-bandit + "BLE", # flake8-blind-except + "PD", # pandas-vet + "PL", # Pylint + "RUF", # Ruff +] + +[lint.per-file-ignores] +"__init__.py" = ["F401"] +"tests/*" = ["ANN", "D", "S101", "PT", "PLR", "PD901"] +"integration_tests/*" = ["ANN", "D", "S101", "PT", "PLR", "PD901"] + +[lint.pydocstyle] +convention = "google" + +#[mccabe] +#max-complexity = 18 +# +# diff --git a/tasks.py b/tasks.py index df21ce2..34ac80d 100644 --- a/tasks.py +++ b/tasks.py @@ -1,8 +1,8 @@ -""" -Tasks for maintaining the project. +"""Tasks for maintaining the project. Execute 'invoke --list' for guidance on using Invoke """ + import platform import webbrowser from pathlib import Path diff --git a/tests/test_batch.py b/tests/test_batch.py index 0469290..ec508f3 100644 --- a/tests/test_batch.py +++ b/tests/test_batch.py @@ -1,4 +1,5 @@ -"""Tests for for the decorator dike.batch""" +"""Tests for for the decorator dike.batch.""" + # pylint: disable=missing-function-docstring import asyncio import importlib @@ -13,16 +14,18 @@ def exceptions_equal(exception1, exception2): - """Returns True if the exceptions have the same type and message""" + """Returns True if the exceptions have the same type and message.""" # pylint: disable=unidiomatic-typecheck - return type(exception1) == type(exception2) and str(exception1) == str(exception2) + return (type(exception1) is type(exception2)) and str(exception1) == str(exception2) async def raise_error(message): raise RuntimeError(message) -@pytest.mark.parametrize("argtype_converter, arg_type_name", [(list, "list"), (np.array, "numpy")]) +@pytest.mark.parametrize( + ("argtype_converter", "arg_type_name"), [(list, "list"), (np.array, "numpy")] +) def test_single_items_batchsize_reached(argtype_converter, arg_type_name): @dike.batch(target_batch_size=3, max_waiting_time=10, argument_type=arg_type_name) async def f(arg1, arg2): @@ -46,7 +49,9 @@ async def run_test(): asyncio.run(run_test()) -@pytest.mark.parametrize("argtype_converter, arg_type_name", [(list, "list"), (np.array, "numpy")]) +@pytest.mark.parametrize( + ("argtype_converter", "arg_type_name"), [(list, "list"), (np.array, "numpy")] +) def test_single_items_kwargs_batchsize_reached(argtype_converter, arg_type_name): @dike.batch(target_batch_size=3, max_waiting_time=10, argument_type=arg_type_name) async def f(arg1, arg2): @@ -70,7 +75,9 @@ async def run_test(): asyncio.run(run_test()) -@pytest.mark.parametrize("argtype_converter, arg_type_name", [(list, "list"), (np.array, "numpy")]) +@pytest.mark.parametrize( + ("argtype_converter", "arg_type_name"), [(list, "list"), (np.array, "numpy")] +) def test_single_items_mixed_kwargs_raises_value_error(argtype_converter, arg_type_name): @dike.batch(target_batch_size=3, max_waiting_time=0.01, argument_type=arg_type_name) async def f(arg1, arg2): @@ -256,7 +263,7 @@ async def run_test(): def test_concurrent_calculations_do_not_clash(): - """Run many calculations in parallel and see if the results are always correct""" + """Run many calculations in parallel and see if the results are always correct.""" @dike.batch(target_batch_size=3, max_waiting_time=0.01) async def f(arg): @@ -266,7 +273,7 @@ async def f(arg): async def do_n_calculations(n): for _ in range(n): number = random.randint(0, 2**20) - result = await (f([number])) + result = await f([number]) assert result[0] == number * 2 await asyncio.sleep(random.random() / 10.0) @@ -278,7 +285,7 @@ async def run_test(): def test_no_numpy_available(monkeypatch): - """Test if without numpy the decorator works normally but refuses to use numpy""" + """Test if without numpy the decorator works normally but refuses to use numpy.""" monkeypatch.setitem(sys.modules, "numpy", None) importlib.reload(dike._batch) # pylint: disable=protected-access diff --git a/tests/test_limit_jobs.py b/tests/test_limit_jobs.py index e736005..939d6e8 100644 --- a/tests/test_limit_jobs.py +++ b/tests/test_limit_jobs.py @@ -1,4 +1,5 @@ -"""Tests for the dike.limit_jobs decorator""" +"""Tests for the dike.limit_jobs decorator.""" + # pylint: disable=missing-function-docstring import asyncio import re @@ -13,14 +14,14 @@ @dike.limit_jobs(limit=2) async def block_until_released(event: asyncio.Event, arg): - """Blocks until the given event is sent and then returns arg""" + """Blocks until the given event is sent and then returns arg.""" await event.wait() return arg @pytest.mark.parametrize("job_limit", [1, 2, 3, 2.5, float("inf")]) def test_simple_usage(job_limit): - """Simply wrap a coroutine with different limits and call it once""" + """Simply wrap a coroutine with different limits and call it once.""" @dike.limit_jobs(limit=job_limit) async def f(): @@ -30,7 +31,7 @@ async def f(): def test_single_calls(): - """Try single calls to the wrapped function""" + """Try single calls to the wrapped function.""" async def testloop(): for i in range(4): @@ -42,7 +43,7 @@ async def testloop(): def test_calls_below_limit(): - """Some simultaneous calls below the threshold should be processed normally""" + """Some simultaneous calls below the threshold should be processed normally.""" async def testloop(): event = asyncio.Event() @@ -58,7 +59,7 @@ async def testloop(): def test_calls_exceeding_limit(): - """Too many simultaneous calls should raise a TooManyCalls exception""" + """Too many simultaneous calls should raise a TooManyCalls exception.""" async def testloop(): event = asyncio.Event() @@ -77,7 +78,7 @@ async def testloop(): def test_call_with_limit_0(): - """Limit of 0 is allowed to create a "forbidden" call""" + """Limit of 0 is allowed to create a "forbidden" call.""" @dike.limit_jobs(limit=0) async def f(): @@ -89,7 +90,7 @@ async def f(): @pytest.mark.parametrize("job_limit", [-1, -1.5, float("-inf")]) def test_unlogical_limits_give_clear_error(job_limit): - """Ensure that a proper error message is shown when trying to set strange limits""" + """Ensure that a proper error message is shown when trying to set strange limits.""" with pytest.raises(ValueError, match=re.escape("Error when wrapping f(). Limit must be >= 0")): @dike.limit_jobs(limit=job_limit) @@ -98,7 +99,7 @@ async def f(): def test_function_instead_coroutine(): - """Ensure that a proper error message is shown when trying to wrap a blocking function""" + """Ensure that a proper error message is shown when trying to wrap a blocking function.""" with pytest.raises(ValueError, match="Error when wrapping .+ Only coroutines can be wrapped"): @dike.limit_jobs(limit=5) diff --git a/tests/test_wrap_in_coroutine.py b/tests/test_wrap_in_coroutine.py index 92f32f1..cbe15ff 100644 --- a/tests/test_wrap_in_coroutine.py +++ b/tests/test_wrap_in_coroutine.py @@ -1,4 +1,5 @@ -"""Tests for the dike.wrap_in_coroutine decorator""" +"""Tests for the dike.wrap_in_coroutine decorator.""" + # pylint: disable=missing-function-docstring import asyncio @@ -6,7 +7,7 @@ def test_wrap_function(): - """Wrap a function without arguments""" + """Wrap a function without arguments.""" @wrap_in_coroutine def f(): @@ -16,7 +17,7 @@ def f(): def test_wrap_function_args(): - """Wrap a function with positional arguments""" + """Wrap a function with positional arguments.""" @wrap_in_coroutine def f(*args): @@ -26,17 +27,17 @@ def f(*args): def test_wrap_function_kargs(): - """Wrap a function with keyword arguments""" + """Wrap a function with keyword arguments.""" @wrap_in_coroutine def f(**kwargs): return kwargs - assert asyncio.run(f(a="A", b=2)) == dict(a="A", b=2) + assert asyncio.run(f(a="A", b=2)) == {"a": "A", "b": 2} def test_wrap_coroutine(): - """Wrap a coroutine without arguments""" + """Wrap a coroutine without arguments.""" @wrap_in_coroutine async def f(): @@ -46,7 +47,7 @@ async def f(): def test_wrap_coroutine_args(): - """Wrap a coroutine with positional arguments""" + """Wrap a coroutine with positional arguments.""" @wrap_in_coroutine async def f(*args): @@ -56,10 +57,10 @@ async def f(*args): def test_wrap_coroutine_kargs(): - """Wrap a coroutine with keyword arguments""" + """Wrap a coroutine with keyword arguments.""" @wrap_in_coroutine async def f(**kwargs): return kwargs - assert asyncio.run(f(a="A", b=2)) == dict(a="A", b=2) + assert asyncio.run(f(a="A", b=2)) == {"a": "A", "b": 2} diff --git a/tox.ini b/tox.ini index 11ecdba..4f764da 100644 --- a/tox.ini +++ b/tox.ini @@ -1,30 +1,13 @@ [tox] isolated_build = true -envlist = lint, test, doctest, build +envlist = test, doctest, build [gh-actions] python = - 3.9: lint, test, doctest, build + 3.9: test, doctest, build 3.8: test 3.7: test -[testenv:lint] -allowlist_externals = - isort - black - flake8 - pylint - yamllint -extras = - test - dev -commands = - isort dike - black dike tests - flake8 dike tests - pylint --output-format=colorized -r y --fail-under 9.0 dike tests - yamllint -d '\{extends: relaxed, ignore: local, rules: \{line-length: disable\}\}' . - [testenv:build] allowlist_externals = poetry