Skip to content

Commit

Permalink
migrate to ruff for linting & formatting (#697)
Browse files Browse the repository at this point in the history
Also advance min python version to 3.8 (from 3.6)
  • Loading branch information
mlin authored Jun 28, 2024
1 parent 04ca37e commit 5ccead0
Show file tree
Hide file tree
Showing 20 changed files with 45 additions and 67 deletions.
16 changes: 8 additions & 8 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,18 @@ jobs:
# https://github.com/marketplace/actions/coveralls-python
uses: AndreMiras/coveralls-python-action@develop

bionic_self_test:
# run_self_test in a ubuntu:18.04 image with only the minimal runtime dependencies installed
focal_self_test:
# run_self_test in a ubuntu:20.04 image with only the minimal runtime dependencies installed
runs-on: ubuntu-20.04
steps:
- uses: actions/checkout@v2
- name: docker build
run: |
mkdir bionic_self_test
cp requirements.txt bionic_self_test
mkdir focal_self_test
cp requirements.txt focal_self_test
cat << 'EOF' > bionic_self_test/Dockerfile
FROM ubuntu:18.04
cat << 'EOF' > focal_self_test/Dockerfile
FROM ubuntu:20.04
ENV LC_ALL C.UTF-8
ENV LANG C.UTF-8
RUN apt-get -qq update && DEBIAN_FRONTEND=noninteractive apt-get -qq install -y python3-pip
Expand All @@ -93,11 +93,11 @@ jobs:
# expectation -- mount miniwdl source tree at /home/wdler/miniwdl
CMD env -C miniwdl python3 -m WDL run_self_test
EOF
docker build -t miniwdl_bionic_self_test bionic_self_test
docker build -t miniwdl_focal_self_test focal_self_test
- name: miniwdl run_self_test
run: |
docker run \
--group-add $(stat -c %g /var/run/docker.sock) -v /var/run/docker.sock:/var/run/docker.sock \
-v $(pwd):/home/wdler/miniwdl -v /tmp:/tmp \
-e AWS_EC2_METADATA_DISABLED=true \
miniwdl_bionic_self_test
miniwdl_focal_self_test
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ The Makefile has a few typical flows:

- `make` or `make test` runs the full test suite with code coverage report (takes several minutes)
- `make qtest` runs most of the tests more quickly (by omitting some slower cases, and not tracking coverage)
- `make pretty` reformats the code with [black](https://github.com/python/black)
- `make check` validates the code with [Pylint](https://www.pylint.org/) and [Pyre](https://pyre-check.org/)
- `make pretty` reformats the code with `ruff format`
- `make check` validates the code with `ruff check` and `mypy`

To quickly run only a relevant subset of the tests, you can e.g. `python3 -m unittest -f tests/test_5stdlib.py` or `python3 -m unittest -f tests.test_5stdlib.TestStdLib.test_glob`.

Expand Down
18 changes: 5 additions & 13 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -36,31 +36,23 @@ singularity_tests:
MINIWDL__SCHEDULER__CONTAINER_BACKEND=singularity \
sh -c 'python3 -m WDL run_self_test && prove -v tests/applied/viral_assemble.t'

ci_housekeeping: sopretty check_check check doc
ci_housekeeping: check_check check doc

ci_unit_tests: unit_tests

check:
ruff check --ignore E741 WDL
mypy WDL
pylint -j `python3 -c 'import multiprocessing as mp; print(mp.cpu_count())'` --errors-only WDL
flake8 WDL
ruff format --check --line-length 100 WDL

check_check:
# regression test against pyre/mypy doing nothing (issue #100)
echo "check_check: str = 42" > WDL/DELETEME_check_check.py
$(MAKE) check > /dev/null 2>&1 && exit 1 || exit 0
rm WDL/DELETEME_check_check.py

# uses black to rewrite source files!
pretty:
black --line-length 100 --target-version py36 WDL/
pylint -d cyclic-import,empty-docstring,missing-docstring,invalid-name,bad-continuation --exit-zero WDL

# for use in CI: complain if source code isn't at a fixed point for black
sopretty:
@git diff --quiet || (echo "ERROR: 'make sopretty' must start with a clean working tree"; exit 1)
$(MAKE) pretty
@git diff --quiet || (echo "ERROR: source files were modified by black; please fix up this commit with 'make pretty'"; exit 1)
ruff format --line-length 100 WDL

# build docker image with current source tree, poised to run tests e.g.:
# docker run --rm -v /var/run/docker.sock:/var/run/docker.sock -v /tmp:/tmp miniwdl
Expand All @@ -87,4 +79,4 @@ doc:

docs: doc

.PHONY: check check_check sopretty pretty test qtest docker doc docs pypi_test pypi bdist ci_housekeeping unit_tests integration_tests skylab_bulk_rna DVGLx viral_assemble
.PHONY: check check_check pretty test qtest docker doc docs pypi_test pypi bdist ci_housekeeping unit_tests integration_tests skylab_bulk_rna DVGLx viral_assemble
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# miniwdl
**[Workflow Description Language](http://openwdl.org/) local runner & developer toolkit for Python 3.6+**
**[Workflow Description Language](http://openwdl.org/) local runner & developer toolkit for Python 3.8+**

![Project Status](https://img.shields.io/badge/status-stable-green.svg)
[![MIT license](https://img.shields.io/badge/license-MIT-brightgreen.svg)](https://github.com/chanzuckerberg/miniwdl/blob/main/LICENSE)
Expand All @@ -9,7 +9,7 @@

## Install miniwdl

Installation requires Python 3.6+, pip3 (or conda) and Docker (or Podman/Singularity/udocker). Linux preferred; [macOS (Intel) compatible with extra steps](https://github.com/chanzuckerberg/miniwdl/issues/145). More detail in [full documentation](https://miniwdl.readthedocs.io/en/latest/getting_started.html).
Installation requires Python 3.8+, pip3 (or conda) and Docker (or Podman/Singularity/udocker). Linux preferred; [macOS (Intel) compatible with extra steps](https://github.com/chanzuckerberg/miniwdl/issues/145). More detail in [full documentation](https://miniwdl.readthedocs.io/en/latest/getting_started.html).

- Install with pip [![PyPI version](https://img.shields.io/pypi/v/miniwdl.svg)](https://pypi.org/project/miniwdl/) : run `pip3 install miniwdl`
- Install with conda [![Anaconda-Server Badge](https://anaconda.org/conda-forge/miniwdl/badges/version.svg)](https://anaconda.org/conda-forge/miniwdl) : run `conda install -c conda-forge miniwdl`
Expand Down
4 changes: 2 additions & 2 deletions WDL/CLI.py
Original file line number Diff line number Diff line change
Expand Up @@ -1586,7 +1586,7 @@ def localize(
)
logging.basicConfig(level=level)
logger = logging.getLogger("miniwdl-localize")
with configure_logger(json=log_json) as set_status:
with configure_logger(json=log_json) as _set_status:
from . import runtime

cfg_arg = None
Expand Down Expand Up @@ -1767,7 +1767,7 @@ def configure(cfg=None, show=False, force=False, **kwargs):
logging.raiseExceptions = False
logging.basicConfig(level=VERBOSE_LEVEL)
logger = logging.getLogger("miniwdl-configure")
with configure_logger() as set_status:
with configure_logger() as _set_status:
if (show or not force) and configure_existing(logger, cfg, always=show):
sys.exit(0)

Expand Down
5 changes: 3 additions & 2 deletions WDL/Lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
The ``descend_imports`` flag controls whether lint warnings are generated for imported documents
recursively (true, default), or otherwise only the given document (false).
"""

import subprocess
import tempfile
import json
Expand Down Expand Up @@ -208,7 +209,7 @@ def _compound_coercion(
if predicates:
return predicates(to_type, from_type)
if not from_type_predicate:
from_type_predicate = lambda ty: not isinstance( # noqa: disable=E731
from_type_predicate = lambda ty: not isinstance( # noqa: E731
ty, (base_to_type, Type.Any)
)
return from_type_predicate(from_type)
Expand Down Expand Up @@ -1167,7 +1168,7 @@ def task(self, obj: Tree.Task) -> Any:
if isinstance(lit, str):
try:
_util.parse_byte_size(lit)
except:
except Exception:
self.add(
obj,
"runtime.memory doesn't follow expected format like '8G' or '1024 MiB'",
Expand Down
4 changes: 1 addition & 3 deletions WDL/StdLib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1037,9 +1037,7 @@ def infer_type(self, expr: "Expr.Apply") -> Type.Base:
expr.arguments[0].typecheck(Type.Array(Type.String()))
arg0ty = expr.arguments[0].type
nonempty = isinstance(arg0ty, Type.Array) and arg0ty.nonempty
return Type.Array(
Type.String(), nonempty=(isinstance(arg0ty, Type.Array) and arg0ty.nonempty)
)
return Type.Array(Type.String(), nonempty=nonempty)

def _call_eager(self, expr: "Expr.Apply", arguments: List[Value.Base]) -> Value.Base:
return Value.Array(
Expand Down
6 changes: 2 additions & 4 deletions WDL/Zip.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ def build_zip_paths(
main_dir: str, wdls: Dict[str, Tree.Document], logger: logging.Logger
) -> Dict[str, str]:
# compute the path inside the archive at which to store each document
import hashlib
import base64

ans = {}
outside_warn = False
Expand Down Expand Up @@ -249,7 +247,7 @@ def unpack(archive_fn: str) -> Iterator[UnpackedZip]:
dn = cleanup.enter_context(tempfile.TemporaryDirectory(prefix="miniwdl_run_zip_"))
try:
shutil.unpack_archive(archive_fn, dn)
except:
except Exception:
raise Error.InputError("Unreadable source archive " + archive_fn)
manifest_fn = os.path.join(dn, "MANIFEST.json")

Expand All @@ -259,7 +257,7 @@ def unpack(archive_fn: str) -> Iterator[UnpackedZip]:
assert isinstance(manifest, dict) and isinstance(
manifest.get("mainWorkflowURL", None), str
)
except:
except Exception:
raise Error.InputError("Missing or invalid MANIFEST.json in " + archive_fn)

dn = os.path.abspath(os.path.dirname(manifest_fn))
Expand Down
6 changes: 2 additions & 4 deletions WDL/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@

import sys
import os
import errno
import inspect
from typing import List, Optional, Callable, Dict, Any, Awaitable, Union
from . import _util, _parser, Error, Type, Value, Env, Expr, Tree, Walker, Zip
from .Tree import (
from . import _util, _parser, Error, Type, Value, Env, Expr, Tree, Walker
from .Tree import ( # noqa: F401
Decl,
StructTypeDef,
Task,
Expand Down
4 changes: 1 addition & 3 deletions WDL/_grammar.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,7 @@

# Development grammar version; any bugfixes to the draft-2/1.0 grammar may need to be forward-
# ported into this.
versions[
"development"
] = r"""
versions["development"] = r"""
///////////////////////////////////////////////////////////////////////////////////////////////////
// document
///////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
5 changes: 2 additions & 3 deletions WDL/_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import time
import fcntl
import shutil
import hashlib
import uuid
from time import sleep
from datetime import datetime
Expand Down Expand Up @@ -212,7 +211,7 @@ def link_force(src: str, dst: str) -> None:

@export
def write_values_json(
values_env: "Env.Bindings[Value.Base]", filename: str, namespace: str = "" # noqa
values_env: "Env.Bindings[Value.Base]", filename: str, namespace: str = ""
) -> None:
from . import values_to_json

Expand Down Expand Up @@ -923,5 +922,5 @@ def currently_in_container() -> bool:
try:
with open(f"/proc/{os.getpid()}/mounts") as infile:
return " / overlay" in infile.read()
except:
except Exception:
return False
7 changes: 3 additions & 4 deletions WDL/runtime/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@
from typing import Union, Dict, Tuple, Any
from .. import Tree, Value, Env
from . import config
from . import task
from . import workflow
from . import _statusbar
from .error import (
from . import task # noqa: F401
from . import workflow # noqa: F401
from .error import ( # noqa: F401
RunFailed,
CommandFailed,
Terminated,
Expand Down
2 changes: 1 addition & 1 deletion WDL/runtime/backend/singularity.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def global_init(cls, cfg: config.Loader, logger: logging.Logger) -> None:
check=True,
universal_newlines=True,
)
except:
except Exception:
raise RuntimeError(
f"Unable to check `{' '.join(cmd)}`; verify Singularity installation"
)
Expand Down
2 changes: 1 addition & 1 deletion WDL/runtime/backend/udocker.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def global_init(cls, cfg: config.Loader, logger: logging.Logger) -> None:
check=True,
universal_newlines=True,
)
except:
except Exception:
raise RuntimeError(f"Unable to check `{' '.join(cmd)}`; verify udocker installation")
logger.notice(
_(
Expand Down
10 changes: 3 additions & 7 deletions WDL/runtime/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def _parse(
raise
try:
return parse(ans)
except:
except Exception:
self._logger.debug(
_(
"failed to parse configuration option",
Expand Down Expand Up @@ -321,11 +321,7 @@ def _strip(value: str) -> str:
if ans:
ans = value.strip()
if len(ans) >= 2 and (
(
ans.startswith("'")
and ans.endswith("'")
or (ans.startswith('"') and ans.endswith('"'))
)
ans.startswith("'") and ans.endswith("'") or (ans.startswith('"') and ans.endswith('"'))
):
ans = ans[1:-1]
return ans
Expand Down Expand Up @@ -370,7 +366,7 @@ def _parse_list(v: str) -> List[Any]:
return ans


def default_plugins() -> "Dict[str,List[importlib_metadata.EntryPoint]]": # type: ignore
def default_plugins() -> "Dict[str,List[importlib_metadata.EntryPoint]]": # type: ignore # noqa: F821
import importlib_metadata # delayed heavy import

return {
Expand Down
2 changes: 1 addition & 1 deletion WDL/runtime/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ def map_paths(v: Value.Base, dn: str) -> Value.Base:
# might otherwise have trouble distinguishing Directory outputs among the
# structured subdirectories we create for compound types.
if isinstance(v, Value.Directory):
with open(os.path.join(dn, ".WDL_Directory"), "w") as dotfile:
with open(os.path.join(dn, ".WDL_Directory"), "w") as _dotfile:
pass
v.value = link
# recurse into compound values
Expand Down
4 changes: 2 additions & 2 deletions WDL/runtime/task_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from typing import Callable, Iterable, Any, Dict, Optional, ContextManager, Set
from abc import ABC, abstractmethod
from contextlib import suppress
from .. import Error, Env, Value, Type
from .. import Error, Value, Type
from .._util import (
TerminationSignalFlag,
path_really_within,
Expand Down Expand Up @@ -283,7 +283,7 @@ def process_runtime(self, logger: logging.Logger, runtime_eval: Dict[str, Value.
elif isinstance(rcv, Value.Array):
try:
ans["returnCodes"] = [v.coerce(Type.Int()).value for v in rcv.value]
except:
except Exception:
pass
if "returnCodes" not in ans:
raise Error.RuntimeError("invalid setting of runtime.returnCodes")
Expand Down
3 changes: 2 additions & 1 deletion WDL/runtime/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,7 @@ def _gather(

class _StdLib(StdLib.Base):
"checks against & updates the file/directory allowlist for the read_* and write_* functions"

cfg: config.Loader
state: StateMachine
cache: CallCache
Expand Down Expand Up @@ -913,7 +914,7 @@ def run_local_workflow(
if not _thread_pools:
# delayed heavy imports -- load .task_container now to work around python issue41567
import importlib_metadata
from .task_container import new as _new_task_container
from .task_container import new as _new_task_container # noqa: F401

assert not _run_id_stack
try:
Expand Down
4 changes: 1 addition & 3 deletions requirements.dev.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# Packages needed for miniwdl development, in addition to those in
# requirements.txt which are needed for miniwdl to run in common use.
-r requirements.txt
black==24.4.2
pylint
sphinx
sphinx-autobuild
sphinx_rtd_theme
Expand All @@ -19,5 +17,5 @@ pytest-xdist
recommonmark
sphinx-argparse
boto3
flake8
mypy
ruff
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
setuptools.setup(
setup_requires=['pbr'],
pbr=True,
python_requires=">3.6.0",
python_requires=">3.8.0",
)

0 comments on commit 5ccead0

Please sign in to comment.