From 9938b789fb692d4157a4643c394559d82543e478 Mon Sep 17 00:00:00 2001 From: maabuu <42468905+maabuu@users.noreply.github.com> Date: Fri, 8 Dec 2023 21:18:43 +0100 Subject: [PATCH 1/3] Fix bug when specifying output --- tests/test_cli.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index f7c3909..b28dd8d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -11,7 +11,18 @@ def test_parse_args() -> None: - args = _parse_args([mol_pred_1ia1, "-l", mol_true_1ia1, "-p", mol_cond_1ia1, "--full-report", "--outfmt", "csv"]) + args = _parse_args( + [ + mol_pred_1ia1, + "-l", + mol_true_1ia1, + "-p", + mol_cond_1ia1, + "--full-report", + "--outfmt", + "csv", + ] + ) assert args.mol_pred[0] == Path(mol_pred_1ia1) assert args.mol_true == Path(mol_true_1ia1) assert args.mol_cond == Path(mol_cond_1ia1) @@ -34,11 +45,7 @@ def test_bust_table() -> None: def test_output() -> None: output = Path(".test_output.csv") - bust( - table=Path(mols_table), - outfmt="csv", - output=output, - ) + bust(table=Path(mols_table), outfmt="csv", output=output) assert output.exists() From 017391074e48ac663a35331a7ed1d09924ca007f Mon Sep 17 00:00:00 2001 From: maabuu <42468905+maabuu@users.noreply.github.com> Date: Tue, 20 Feb 2024 10:40:12 +0000 Subject: [PATCH 2/3] Switch to ruff --- .pre-commit-config.yaml | 161 ++++++++++++++++++---------------------- .vscode/launch.json | 61 +++++++-------- .vscode/settings.json | 18 ++--- pyproject.toml | 20 +++-- 4 files changed, 122 insertions(+), 138 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3faf0ce..71a15d3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,100 +5,87 @@ ci: exclude: ^tests/conftest/|^.vscode/|^posebusters/datasets/pdb repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.5.0 + hooks: + # - id: no-commit-to-branch # branch protected on github + - id: forbid-submodules + - id: check-added-large-files + - id: check-case-conflict + - id: check-docstring-first + - id: check-merge-conflict + - id: check-symlinks + - id: check-yaml + - id: check-toml + - id: check-json + - id: check-ast + - id: debug-statements + - id: destroyed-symlinks + - id: detect-private-key + - id: end-of-file-fixer + - id: mixed-line-ending + - id: requirements-txt-fixer + - id: trailing-whitespace + - id: name-tests-test + args: [--pytest-test-first] + - id: sort-simple-yaml -- repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.5.0 - hooks: - # - id: no-commit-to-branch # branch protected on github - - id: forbid-submodules - - id: check-added-large-files - - id: check-case-conflict - - id: check-docstring-first - - id: check-merge-conflict - - id: check-symlinks - - id: check-yaml - - id: check-toml - - id: check-json - - id: check-ast - - id: debug-statements - - id: destroyed-symlinks - - id: detect-private-key - - id: end-of-file-fixer - - id: mixed-line-ending - - id: requirements-txt-fixer - - id: trailing-whitespace - - id: name-tests-test - args: [--pytest-test-first] - - id: sort-simple-yaml - -- repo: https://github.com/pre-commit/pygrep-hooks - rev: v1.10.0 - hooks: - - id: python-check-blanket-noqa - - id: python-check-blanket-type-ignore - - id: python-no-log-warn - - id: python-no-eval - - id: python-use-type-annotations - - id: rst-backticks - - id: rst-directive-colons - - id: rst-inline-touching-normal - - id: text-unicode-replacement-char - -# update syntax -- repo: https://github.com/asottile/pyupgrade - rev: v3.15.0 - hooks: - - id: pyupgrade - args: ["--py37-plus"] - -# sort imports -- repo: https://github.com/PyCQA/isort - rev: 5.12.0 - hooks: - - id: isort - args: ["--settings-path=pyproject.toml"] + - repo: https://github.com/pre-commit/pygrep-hooks + rev: v1.10.0 + hooks: + - id: python-check-blanket-noqa + - id: python-check-blanket-type-ignore + - id: python-no-log-warn + - id: python-no-eval + - id: python-use-type-annotations + - id: rst-backticks + - id: rst-directive-colons + - id: rst-inline-touching-normal + - id: text-unicode-replacement-char -# find unused imports -- repo: https://github.com/hadialqattan/pycln - rev: v2.4.0 - hooks: - - id: pycln - args: ["--config=pyproject.toml"] - stages: ["manual"] + # update syntax + - repo: https://github.com/asottile/pyupgrade + rev: v3.15.0 + hooks: + - id: pyupgrade + args: ["--py37-plus"] -# format code -- repo: https://github.com/psf/black - rev: 23.11.0 - hooks: - - id: black - language_version: python3.11 - args: ["--config=pyproject.toml"] + - repo: https://github.com/astral-sh/ruff-pre-commit + # Ruff version. + rev: v0.2.2 + hooks: + # Run the linter. + - id: ruff + args: [--fix] + # Run the formatter. + - id: ruff-format -# lint code -- repo: https://github.com/pycqa/flake8 - rev: 6.1.0 - hooks: - - id: flake8 - additional_dependencies: ["flake8-black", "flake8-docstrings", "flake8-pyproject"] + # find unused imports + - repo: https://github.com/hadialqattan/pycln + rev: v2.4.0 + hooks: + - id: pycln + args: ["--config=pyproject.toml"] + stages: ["manual"] -# check types -- repo: https://github.com/pre-commit/mirrors-mypy + # check types + - repo: https://github.com/pre-commit/mirrors-mypy rev: v1.7.0 hooks: - - id: mypy + - id: mypy additional_dependencies: ["types-PyYAML"] -# find typos -- repo: https://github.com/codespell-project/codespell - rev: v2.2.6 - hooks: - - id: codespell - additional_dependencies: ["tomli"] + # find typos + - repo: https://github.com/codespell-project/codespell + rev: v2.2.6 + hooks: + - id: codespell + additional_dependencies: ["tomli"] -# security lint -- repo: https://github.com/PyCQA/bandit - rev: 1.7.5 - hooks: - - id: bandit - args: ["-c", "pyproject.toml"] - additional_dependencies: ["bandit[toml]"] + # security lint + - repo: https://github.com/PyCQA/bandit + rev: 1.7.5 + hooks: + - id: bandit + args: ["-c", "pyproject.toml"] + additional_dependencies: ["bandit[toml]"] diff --git a/.vscode/launch.json b/.vscode/launch.json index b58d886..66e3156 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -4,9 +4,26 @@ // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 "version": "0.2.0", "configurations": [ + { + "name": "Python: Debug Tests", + "type": "debugpy", + "request": "launch", + "program": "${file}", + "purpose": [ + "debug-test" + ], + "console": "integratedTerminal", + "presentation": { + "hidden": true + }, + "justMyCode": false, + "env": { + "PYTEST_ADDOPTS": "--no-cov" + } + }, { "name": "bust conditioned on protein and ligand", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -22,7 +39,7 @@ }, { "name": "bust conditioned on protein and ligand -- long", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -40,7 +57,7 @@ }, { "name": "bust conditioned on protein", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -54,7 +71,7 @@ }, { "name": "bust unconditioned", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -66,7 +83,7 @@ }, { "name": "bust unconditioned - yael test case", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -78,7 +95,7 @@ }, { "name": "bust table input", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -91,7 +108,7 @@ }, { "name": "bust table -- long", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -106,7 +123,7 @@ }, { "name": "bust table -- csv", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -119,25 +136,9 @@ "csv", ] }, - { - "name": "Python: Debug Tests", - "type": "python", - "request": "launch", - "program": "${file}", - "cwd": "${workspaceFolder}", - "purpose": [ - "debug-test" - ], - "stopOnEntry": false, - "console": "integratedTerminal", - "justMyCode": true, - "env": { - "PYTEST_ADDOPTS": "--no-cov" - } - }, { "name": "bust conditioned on protein and ligand -- 1ia1", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -156,7 +157,7 @@ }, { "name": "bust conditioned on protein -- 1ia1", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -170,7 +171,7 @@ }, { "name": "bust conditioned on protein -- 1w1p crystal", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -189,7 +190,7 @@ }, { "name": "bust conditioned on protein -- 1g9v gold", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -210,7 +211,7 @@ }, { "name": "bust conditioned on protein -- 1gkc", - "type": "python", + "type": "debugpy", "request": "launch", "cwd": "${workspaceFolder}", "module": "posebusters", @@ -228,6 +229,6 @@ "--outfmt", "long", ] - } + }, ] } \ No newline at end of file diff --git a/.vscode/settings.json b/.vscode/settings.json index fcae8a3..34a124b 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -8,27 +8,19 @@ "editor.defaultFormatter": "vscode.json-language-features" }, "[python]": { - "editor.defaultFormatter": "ms-python.black-formatter", + "editor.defaultFormatter": "charliermarsh.ruff", "editor.formatOnSave": true, "editor.codeActionsOnSave": { - "source.organizeImports": true + "source.fixAll": "explicit", + "source.organizeImports": "explicit" }, }, "python.analysis.typeCheckingMode": "basic", "python.defaultInterpreterPath": "/usr/local/bin/python", "python.testing.pytestEnabled": true, "python.testing.unittestEnabled": false, - "black-formatter.args": [ - "--config=pyproject.toml" - ], - "flake8.args": [ - "--toml-config=pyproject.toml" - ], - "isort.args": [ - "--settings-path=pyproject.toml" - ], - "pylint.args": [ - "--rcfile=pyproject.toml" + "python.testing.pytestArgs": [ + "--no-cov" ], "autoDocstring.docstringFormat": "google-notypes", "autoDocstring.generateDocstringOnEnter": true, diff --git a/pyproject.toml b/pyproject.toml index 5de09aa..e73386b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,7 +28,7 @@ dynamic = ["version", "description"] dependencies = ['rdkit >= 2020.09', 'pandas', 'numpy', 'pyyaml'] [project.optional-dependencies] -dev = ["isort", "black"] +dev = ["ruff"] docs = [ "ipython", "nbsphinx", @@ -50,12 +50,14 @@ bust = "posebusters.cli:main" [tool.flit.module] name = "posebusters" -[tool.isort] -profile = "black" -src_paths = ["posebusters", "tests"] - -[tool.black] +[tool.ruff] line-length = 120 +show-fixes = true +lint.select = ["F", "W", "E", "I", "UP", "PL", "T201"] +lint.ignore = ["E501"] + +[tool.ruff.format] +docstring-code-format = true [tool.mypy] ignore_missing_imports = true @@ -67,7 +69,7 @@ per-file-ignores = """ tests/*: D """ -[tool.pylint] +[tool.pylint.main] max-line-length = 120 [tool.coverage.run] @@ -75,10 +77,12 @@ branch = true [tool.coverage.report] fail_under = 100 +show_missing = true +exclude_lines = ["pragma: no cover", "raise NotImplementedError"] [tool.pytest.ini_options] addopts = "--cov-report xml:coverage.xml --cov posebusters --cov-fail-under 0 --cov-append -m 'not integration'" -pythonpath = "posebusters" +pythonpath = ["posebusters"] testpaths = "tests" [tool.pycln] From 045071ab18b3c832e4be25269aa6cf1bd38f1f63 Mon Sep 17 00:00:00 2001 From: maabuu <42468905+maabuu@users.noreply.github.com> Date: Tue, 20 Feb 2024 11:08:20 +0000 Subject: [PATCH 3/3] Cleanup ruff linting errors --- posebusters/cli.py | 2 +- posebusters/modules/distance_geometry.py | 6 +++--- .../modules/intermolecular_distance.py | 2 +- posebusters/modules/rmsd.py | 2 +- posebusters/modules/volume_overlap.py | 2 +- posebusters/posebusters.py | 10 +++++----- posebusters/tools/loading.py | 14 +++++++------- posebusters/tools/molecules.py | 19 +++---------------- posebusters/tools/protein.py | 2 +- pyproject.toml | 3 +++ tests/test_posebusters.py | 8 ++++---- 11 files changed, 30 insertions(+), 40 deletions(-) diff --git a/posebusters/cli.py b/posebusters/cli.py index 7e573fc..cd0b6e9 100644 --- a/posebusters/cli.py +++ b/posebusters/cli.py @@ -27,7 +27,7 @@ def main(): logger.error(e) -def bust( +def bust( # noqa: PLR0913 mol_pred: list[Path | Mol] = [], mol_true: Path | Mol | None = None, mol_cond: Path | Mol | None = None, diff --git a/posebusters/modules/distance_geometry.py b/posebusters/modules/distance_geometry.py index 121ab9f..18043d6 100644 --- a/posebusters/modules/distance_geometry.py +++ b/posebusters/modules/distance_geometry.py @@ -64,7 +64,7 @@ } -def check_geometry( +def check_geometry( # noqa: PLR0913, PLR0915 mol_pred: Mol, threshold_bad_bond_length: float = 0.2, threshold_clash: float = 0.2, @@ -234,8 +234,8 @@ def _two_bonds_to_angle(bond1: tuple[int, int], bond2: tuple[int, int]) -> None set1 = set(bond1) set2 = set(bond2) all_atoms = set1 | set2 - # angle requires two bonds to share exactly one atom - if len(all_atoms) != 3: + # angle requires two bonds to share exactly one atom, that is we must have 3 atoms + if len(all_atoms) != 3: # noqa: PLR2004 return None # find shared atom shared_atom = set1 & set2 diff --git a/posebusters/modules/intermolecular_distance.py b/posebusters/modules/intermolecular_distance.py index e7a8a97..8730a39 100644 --- a/posebusters/modules/intermolecular_distance.py +++ b/posebusters/modules/intermolecular_distance.py @@ -12,7 +12,7 @@ _periodic_table = GetPeriodicTable() -def check_intermolecular_distance( +def check_intermolecular_distance( # noqa: PLR0913 mol_pred: Mol, mol_cond: Mol, radius_type: str = "vdw", diff --git a/posebusters/modules/rmsd.py b/posebusters/modules/rmsd.py index 24e0d99..6b7c52b 100644 --- a/posebusters/modules/rmsd.py +++ b/posebusters/modules/rmsd.py @@ -78,7 +78,7 @@ def check_rmsd( return {"results": results} -def robust_rmsd( +def robust_rmsd( # noqa: PLR0913 mol_probe: Mol, mol_ref: Mol, conf_id_probe: int = -1, diff --git a/posebusters/modules/volume_overlap.py b/posebusters/modules/volume_overlap.py index dc5cd3c..4d7a519 100644 --- a/posebusters/modules/volume_overlap.py +++ b/posebusters/modules/volume_overlap.py @@ -13,7 +13,7 @@ logger = logging.getLogger(__name__) -def check_volume_overlap( +def check_volume_overlap( # noqa: PLR0913 mol_pred: Mol, mol_cond: Mol, clash_cutoff: float = 0.05, diff --git a/posebusters/posebusters.py b/posebusters/posebusters.py index 60e1694..e6059f5 100644 --- a/posebusters/posebusters.py +++ b/posebusters/posebusters.py @@ -131,7 +131,7 @@ def _run(self) -> Generator[dict, None, None]: """ self._initialize_modules() - for i, paths in self.file_paths.iterrows(): + for _, paths in self.file_paths.iterrows(): mol_args = {} if "mol_cond" in paths and paths["mol_cond"] is not None: mol_cond_load_params = self.config.get("loading", {}).get("mol_cond", {}) @@ -151,15 +151,15 @@ def _run(self) -> Generator[dict, None, None]: for name, fname, func, args in zip(self.module_name, self.fname, self.module_func, self.module_args): # pick needed arguments for module - args = {k: v for k, v in mol_args.items() if k in args} + args_needed = {k: v for k, v in mol_args.items() if k in args} # loading takes all inputs if fname == "loading": - args = {k: args.get(k, None) for k in args} + args_needed = {k: args_needed.get(k, None) for k in args_needed} # run module when all needed input molecules are valid Mol objects - if fname != "loading" and not all(args.get(m, None) for m in args): + if fname != "loading" and not all(args_needed.get(m, None) for m in args_needed): module_output: dict[str, Any] = {"results": {}} else: - module_output = func(**args) + module_output = func(**args_needed) # save to object self.results[results_key].extend([(name, k, v) for k, v in module_output["results"].items()]) diff --git a/posebusters/tools/loading.py b/posebusters/tools/loading.py index 05a2526..66ccfde 100644 --- a/posebusters/tools/loading.py +++ b/posebusters/tools/loading.py @@ -68,15 +68,15 @@ def safe_supply_mols(path: Path, load_all=True, sanitize=True, **load_params) -> supplier = SDMolSupplier(str(path), sanitize=False, strictParsing=True) i = 0 for mol in supplier: - mol = _process_mol(mol, sanitize=sanitize, **load_params) + mol_clean = _process_mol(mol, sanitize=sanitize, **load_params) i += 1 - if mol is not None: - mol.SetProp("_Path", str(path)) - mol.SetProp("_Index", str(i)) - yield mol + if mol_clean is not None: + mol_clean.SetProp("_Path", str(path)) + mol_clean.SetProp("_Index", str(i)) + yield mol_clean -def _load_mol( +def _load_mol( # noqa: PLR0913 path: Path, load_all=False, sanitize=False, @@ -136,7 +136,7 @@ def _load_and_combine_mols(path: Path, sanitize=True, removeHs=True, strictParsi return mol -def _process_mol( +def _process_mol( # noqa: PLR0913 mol: Mol | None, smiles: str | None = None, cleanup=False, diff --git a/posebusters/tools/molecules.py b/posebusters/tools/molecules.py index 013806a..678642c 100644 --- a/posebusters/tools/molecules.py +++ b/posebusters/tools/molecules.py @@ -9,23 +9,10 @@ from rdkit import RDLogger from rdkit.Chem.AllChem import AssignBondOrdersFromTemplate from rdkit.Chem.Lipinski import HAcceptorSmarts, HDonorSmarts -from rdkit.Chem.rdchem import ( - AtomValenceException, - Bond, - Conformer, - GetPeriodicTable, - Mol, - RWMol, -) +from rdkit.Chem.rdchem import AtomValenceException, Bond, Conformer, GetPeriodicTable, Mol, RWMol from rdkit.Chem.rdMolAlign import GetBestAlignmentTransform from rdkit.Chem.rdmolfiles import MolFromSmarts -from rdkit.Chem.rdmolops import ( - AddHs, - RemoveHs, - RemoveStereochemistry, - RenumberAtoms, - SanitizeMol, -) +from rdkit.Chem.rdmolops import AddHs, RemoveHs, RemoveStereochemistry, RenumberAtoms, SanitizeMol from rdkit.Chem.rdMolTransforms import TransformConformer logger = getLogger(__name__) @@ -184,7 +171,7 @@ def _get_atomic_number(atomic_symbol: str): symbol = "H" return _periodic_table.GetAtomicNumber(symbol) except Exception: - print(atomic_symbol) + logger.error("Unknown atomic symbol: %s", atomic_symbol) return atomic_symbol diff --git a/posebusters/tools/protein.py b/posebusters/tools/protein.py index ac4b885..4f7d270 100644 --- a/posebusters/tools/protein.py +++ b/posebusters/tools/protein.py @@ -62,7 +62,7 @@ def get_atom_type_mask(mol: Mol, ignore_types: Iterable[str]) -> list[bool]: ] -def _keep_atom( +def _keep_atom( # noqa: PLR0913, PLR0911 atom: Atom, ignore_h: bool, ignore_protein: bool, ignore_org_cof: bool, ignore_inorg_cof: bool, ignore_water: bool ) -> bool: """Whether to keep atom for given ignore flags.""" diff --git a/pyproject.toml b/pyproject.toml index e73386b..b6d9fc2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,6 +59,9 @@ lint.ignore = ["E501"] [tool.ruff.format] docstring-code-format = true +[tool.ruff.lint.per-file-ignores] +"tests/*" = ["D", "PLR2004"] + [tool.mypy] ignore_missing_imports = true diff --git a/tests/test_posebusters.py b/tests/test_posebusters.py index 73e9cdd..5177c94 100644 --- a/tests/test_posebusters.py +++ b/tests/test_posebusters.py @@ -65,13 +65,13 @@ def test_bust_mol_rdkit() -> None: assert df.all(axis=1).values[0] -def test_bust_mols_hydrogen() -> None: +def test_bust_mols_hydrogen(threshold=8) -> None: posebusters = PoseBusters("mol") df = posebusters.bust([mol_single_h]) - assert df.sum(axis=1).values[0] >= 8 # energy ratio test fails + assert df.sum(axis=1).values[0] >= threshold # energy ratio test fails -def test_bust_mols_consistency() -> None: +def test_bust_mols_consistency(atol=1e-6) -> None: # check that running the same molecule twice gives the same result posebusters = PoseBusters("mol") @@ -84,4 +84,4 @@ def test_bust_mols_consistency() -> None: for v1, v2 in zip(result_2.values, result_3.values): if v1[2] == v2[2] or math.isnan(v1[2]): continue - assert abs(v1[2] - v2[2]) < 1e-6, f"{v1[0], v1[1]}: {v1[2]} != {v2[2]}" + assert abs(v1[2] - v2[2]) < atol, f"{v1[0], v1[1]}: {v1[2]} != {v2[2]}"