From cd5e1a8a70bcd6b53a9a4e45f879ed0f39abb335 Mon Sep 17 00:00:00 2001 From: Kevin Boyd Date: Fri, 18 Sep 2026 11:23:40 -0400 Subject: [PATCH 1/5] Enable Ruff async return and simplify checks --- benchmarks/tfd_bench.py | 6 +----- nvmolkit/tests/test_mcs.py | 8 ++++---- pyproject.toml | 28 ++++++++++++++++------------ 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/benchmarks/tfd_bench.py b/benchmarks/tfd_bench.py index 7f34798a..1dbbb8c7 100644 --- a/benchmarks/tfd_bench.py +++ b/benchmarks/tfd_bench.py @@ -187,11 +187,7 @@ def verify_correctness(mol: Chem.Mol, tolerance: float = 0.01) -> bool: if len(rdkit_result) != len(nvmol_result): return False - for rd, nv in zip(rdkit_result, nvmol_result): - if abs(rd - nv) > tolerance: - return False - - return True + return all(abs(rd - nv) <= tolerance for rd, nv in zip(rdkit_result, nvmol_result, strict=True)) def load_pkl_files(pkl_paths: List[str]) -> List[Chem.Mol]: diff --git a/nvmolkit/tests/test_mcs.py b/nvmolkit/tests/test_mcs.py index 5bbff38d..ee9f335b 100644 --- a/nvmolkit/tests/test_mcs.py +++ b/nvmolkit/tests/test_mcs.py @@ -90,13 +90,13 @@ def _assert_result_storage(result, mol_table): assert item.bond_mapping.shape == (item.num_bonds, 2) if item.num_atoms: - assert np.all((0 <= item.atom_mapping[:, 0]) & (item.atom_mapping[:, 0] < mol_table[idx_a].GetNumAtoms())) - assert np.all((0 <= item.atom_mapping[:, 1]) & (item.atom_mapping[:, 1] < mol_table[idx_b].GetNumAtoms())) + assert np.all((item.atom_mapping[:, 0] >= 0) & (item.atom_mapping[:, 0] < mol_table[idx_a].GetNumAtoms())) + assert np.all((item.atom_mapping[:, 1] >= 0) & (item.atom_mapping[:, 1] < mol_table[idx_b].GetNumAtoms())) assert len(np.unique(item.atom_mapping[:, 0])) == item.num_atoms assert len(np.unique(item.atom_mapping[:, 1])) == item.num_atoms if item.num_bonds: - assert np.all((0 <= item.bond_mapping[:, 0]) & (item.bond_mapping[:, 0] < mol_table[idx_a].GetNumBonds())) - assert np.all((0 <= item.bond_mapping[:, 1]) & (item.bond_mapping[:, 1] < mol_table[idx_b].GetNumBonds())) + assert np.all((item.bond_mapping[:, 0] >= 0) & (item.bond_mapping[:, 0] < mol_table[idx_a].GetNumBonds())) + assert np.all((item.bond_mapping[:, 1] >= 0) & (item.bond_mapping[:, 1] < mol_table[idx_b].GetNumBonds())) assert len(np.unique(item.bond_mapping[:, 0])) == item.num_bonds assert len(np.unique(item.bond_mapping[:, 1])) == item.num_bonds diff --git a/pyproject.toml b/pyproject.toml index b2abc3b4..4731c495 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -149,21 +149,25 @@ ignore = [ "D100", # Checks for undocumented public module definitions. "E501", # Checks for lines that exceed the specified maximum character length. "E741", # Checks for the use of the characters 'l', 'O', or 'I' as variable names. + "RET505", # Preserve elif chains after terminating branches. "RUF005", # Checks for uses of the + operator to concatenate collections. ] select = [ - "A", # Builtin shadowing - "C", # C-prefixed convention checks - "D", # Documentation formatting - "DTZ", # Datetime timezone checks - "E", # Style and whitespace checks - "F", # Pyflakes correctness checks - "I", # Import sorting - "LOG", # Logging checks - "PIE", # Miscellaneous correctness checks - "PLE", # Pylint errors - "RUF", # Ruff-specific checks - "W", # Pycodestyle warnings + "A", # Builtin shadowing + "ASYNC", # Async correctness checks + "C", # C-prefixed convention checks + "D", # Documentation formatting + "DTZ", # Datetime timezone checks + "E", # Style and whitespace checks + "F", # Pyflakes correctness checks + "I", # Import sorting + "LOG", # Logging checks + "PIE", # Miscellaneous correctness checks + "PLE", # Pylint errors + "RET", # Return statement checks + "RUF", # Ruff-specific checks + "SIM", # Simplification checks + "W", # Pycodestyle warnings ] [tool.ruff.lint.per-file-ignores] From 8a18ccdde0908c37ba0a303d40bbbf857461deb0 Mon Sep 17 00:00:00 2001 From: Kevin Boyd Date: Fri, 18 Sep 2026 11:29:32 -0400 Subject: [PATCH 2/5] Enable substantive Ruff bugbear checks --- benchmarks/butina_clustering_bench.py | 10 +++++++--- benchmarks/conformer_rmsd_bench.py | 2 +- benchmarks/etkdg_bench.py | 2 +- benchmarks/ff_optimize_bench.py | 2 +- benchmarks/tfd_bench.py | 6 +++--- nvmolkit/tests/test_tfd.py | 6 +++--- pyproject.toml | 2 ++ 7 files changed, 18 insertions(+), 12 deletions(-) diff --git a/benchmarks/butina_clustering_bench.py b/benchmarks/butina_clustering_bench.py index e3a0b6cf..c7875b08 100644 --- a/benchmarks/butina_clustering_bench.py +++ b/benchmarks/butina_clustering_bench.py @@ -273,7 +273,7 @@ def save_results(): if "fused" in runs: print(f"Running fused_butina size {size} cutoff {cutoff}") fused_result = time_it( - lambda: fused_butina( + lambda fps_mat=fps_mat, cutoff=cutoff: fused_butina( fps_mat, cutoff=cutoff, metric="tanimoto", @@ -297,7 +297,9 @@ def save_results(): f"reordering {nvmol_reordering}" ) nvmolkit_cluster_only_result = time_it( - lambda: bench_nvmol_inner(dist_mat, cutoff, max_nl, nvmol_reordering), + lambda dist_mat=dist_mat, cutoff=cutoff, max_nl=max_nl, nvmol_reordering=nvmol_reordering: ( + bench_nvmol_inner(dist_mat, cutoff, max_nl, nvmol_reordering) + ), gpu_sync=True, runs=n_runs, ) @@ -308,7 +310,9 @@ def save_results(): if nvmol_reordering: print(f"Running nvmolkit_with_tanimoto size {size} cutoff {cutoff} max_nl {max_nl}") nvmolkit_with_tanimoto_result = time_it( - lambda: bench_nvmol_with_tanimoto(fps_mat, cutoff, max_nl), + lambda fps_mat=fps_mat, cutoff=cutoff, max_nl=max_nl: bench_nvmol_with_tanimoto( + fps_mat, cutoff, max_nl + ), gpu_sync=True, runs=n_runs, ) diff --git a/benchmarks/conformer_rmsd_bench.py b/benchmarks/conformer_rmsd_bench.py index 8ce21934..c3889220 100644 --- a/benchmarks/conformer_rmsd_bench.py +++ b/benchmarks/conformer_rmsd_bench.py @@ -232,7 +232,7 @@ def run( gpu_pairs_per_s: float | None = None if not no_nvmolkit: print(" nvMolKit GPU (batched):") - result = time_it(lambda: bench_gpu_batch(mols), runs=5, warmups=2, gpu_sync=True) + result = time_it(lambda mols=mols: bench_gpu_batch(mols), runs=5, warmups=2, gpu_sync=True) gpu_time_s = result.median_s gpu_std_s = result.std_ms / 1000.0 gpu_pairs_per_s = total_pairs / gpu_time_s diff --git a/benchmarks/etkdg_bench.py b/benchmarks/etkdg_bench.py index ba42250e..b0b48151 100644 --- a/benchmarks/etkdg_bench.py +++ b/benchmarks/etkdg_bench.py @@ -546,7 +546,7 @@ def main() -> None: rdkit_throughput_per_s = throughput_per_s( rdkit_processed_count * args.confs_per_mol, results["rdkit"][0].mean_ms ) - for name, (timing, run_mols) in results.items(): + for name, (timing, _run_mols) in results.items(): speedup = "" if rdkit_throughput_per_s is not None and name != "rdkit" and timing.mean_ms > 0: method_throughput = throughput_per_s(len(mols) * args.confs_per_mol, timing.mean_ms) diff --git a/benchmarks/ff_optimize_bench.py b/benchmarks/ff_optimize_bench.py index d1c6f88a..af0d752e 100644 --- a/benchmarks/ff_optimize_bench.py +++ b/benchmarks/ff_optimize_bench.py @@ -587,7 +587,7 @@ def main() -> None: applied_num_gpus = args.num_gpus csv_rows: list[dict[str, object]] = [] - for name, (avg_ms, std_ms, energies) in results.items(): + for name, (avg_ms, std_ms, _energies) in results.items(): is_nv = name == "nvmolkit" is_rdkit = name == "rdkit" batch_size = applied_batch_size if is_nv else "N/A" diff --git a/benchmarks/tfd_bench.py b/benchmarks/tfd_bench.py index 1dbbb8c7..2eef306a 100644 --- a/benchmarks/tfd_bench.py +++ b/benchmarks/tfd_bench.py @@ -330,19 +330,19 @@ def run_benchmarks( result["rdkit_molecules_processed"] = None if not skip_nvmolkit: - timing = time_it(lambda: bench_nvmol_gpu_list(mols), runs=runs, warmups=warmups) + timing = time_it(lambda mols=mols: bench_nvmol_gpu_list(mols), runs=runs, warmups=warmups) t, s = timing.mean_ms, timing.std_ms result["nvmol_gpu_list_time_ms"] = t result["nvmol_gpu_list_std_ms"] = s print(f" nvMolKit (GPU list): {t:8.2f} ms (+/- {s:.2f})") - timing = time_it(lambda: bench_nvmol_gpu_numpy(mols), runs=runs, warmups=warmups) + timing = time_it(lambda mols=mols: bench_nvmol_gpu_numpy(mols), runs=runs, warmups=warmups) t, s = timing.mean_ms, timing.std_ms result["nvmol_gpu_numpy_time_ms"] = t result["nvmol_gpu_numpy_std_ms"] = s print(f" nvMolKit (GPU numpy): {t:8.2f} ms (+/- {s:.2f})") - timing = time_it(lambda: bench_nvmol_gpu_tensor(mols), runs=runs, warmups=warmups) + timing = time_it(lambda mols=mols: bench_nvmol_gpu_tensor(mols), runs=runs, warmups=warmups) t, s = timing.mean_ms, timing.std_ms result["nvmol_gpu_tensor_time_ms"] = t result["nvmol_gpu_tensor_std_ms"] = s diff --git a/nvmolkit/tests/test_tfd.py b/nvmolkit/tests/test_tfd.py index 03560575..4182992f 100644 --- a/nvmolkit/tests/test_tfd.py +++ b/nvmolkit/tests/test_tfd.py @@ -111,7 +111,7 @@ def test_invalid_maxdev_raises(self, simple_mol_with_conformers): """Test that invalid maxDev raises error.""" mol = simple_mol_with_conformers - with pytest.raises(Exception): + with pytest.raises(ValueError): tfd.GetTFDMatrix(mol, maxDev="invalid") @@ -449,12 +449,12 @@ def test_large_molecule(self): def test_invalid_molecule_raises(self): """Test that None molecule raises error.""" - with pytest.raises(Exception): + with pytest.raises(ValueError): tfd.GetTFDMatrix(None) def test_invalid_molecule_in_batch_raises(self, simple_mol_with_conformers): """Test that None in batch raises error.""" mols = [simple_mol_with_conformers, None] - with pytest.raises(Exception): + with pytest.raises(ValueError): tfd.GetTFDMatrices(mols) diff --git a/pyproject.toml b/pyproject.toml index 4731c495..d9c5c514 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -145,6 +145,7 @@ include = ["nvmolkit/**/*.py", "benchmarks/**/*.py", "setup.py"] [tool.ruff.lint] ignore = [ + "B905", # Allow zip() to truncate to the shortest input. "C901", # Checks for functions with a high McCabe complexity. "D100", # Checks for undocumented public module definitions. "E501", # Checks for lines that exceed the specified maximum character length. @@ -155,6 +156,7 @@ ignore = [ select = [ "A", # Builtin shadowing "ASYNC", # Async correctness checks + "B", # Bugbear correctness checks "C", # C-prefixed convention checks "D", # Documentation formatting "DTZ", # Datetime timezone checks From 1bc835aaf0cb22f43bce55268e0a7653f3c0ecf1 Mon Sep 17 00:00:00 2001 From: Kevin Boyd Date: Fri, 18 Sep 2026 11:33:56 -0400 Subject: [PATCH 3/5] Enable Ruff performance checks --- nvmolkit/mcs.py | 7 ++----- pyproject.toml | 2 ++ 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/nvmolkit/mcs.py b/nvmolkit/mcs.py index c7dca2a0..c87e751c 100644 --- a/nvmolkit/mcs.py +++ b/nvmolkit/mcs.py @@ -214,13 +214,10 @@ def _all_pairs(num_mols: int, upper_triangle: bool, include_diagonal: bool) -> t if upper_triangle: for i in range(num_mols): begin = i if include_diagonal else i + 1 - for j in range(begin, num_mols): - pairs.append((i, j)) + pairs.extend((i, j) for j in range(begin, num_mols)) else: for i in range(num_mols): - for j in range(num_mols): - if include_diagonal or i != j: - pairs.append((i, j)) + pairs.extend((i, j) for j in range(num_mols) if include_diagonal or i != j) return tuple(pairs) diff --git a/pyproject.toml b/pyproject.toml index d9c5c514..7d302b9d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -150,6 +150,7 @@ ignore = [ "D100", # Checks for undocumented public module definitions. "E501", # Checks for lines that exceed the specified maximum character length. "E741", # Checks for the use of the characters 'l', 'O', or 'I' as variable names. + "PERF203", # Preserve per-conformer exception isolation in benchmark energy collection. "RET505", # Preserve elif chains after terminating branches. "RUF005", # Checks for uses of the + operator to concatenate collections. ] @@ -164,6 +165,7 @@ select = [ "F", # Pyflakes correctness checks "I", # Import sorting "LOG", # Logging checks + "PERF", # Performance anti-pattern checks "PIE", # Miscellaneous correctness checks "PLE", # Pylint errors "RET", # Return statement checks From 7dfa24cf1046862388ed4181bd03c7c7643910ff Mon Sep 17 00:00:00 2001 From: Kevin Boyd Date: Fri, 18 Sep 2026 11:39:30 -0400 Subject: [PATCH 4/5] Enable Ruff pylint warning checks --- nvmolkit/tests/test_batched_forcefield.py | 12 ++++++------ nvmolkit/tests/test_skill.py | 1 + nvmolkit/tests/test_substructure.py | 14 +++++++------- pyproject.toml | 5 ++++- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/nvmolkit/tests/test_batched_forcefield.py b/nvmolkit/tests/test_batched_forcefield.py index 377e0ac7..210e80ae 100644 --- a/nvmolkit/tests/test_batched_forcefield.py +++ b/nvmolkit/tests/test_batched_forcefield.py @@ -411,8 +411,8 @@ def test_mmff_batched_forcefield_multi_conformer_matches_rdkit(): @pytest.mark.parametrize( "ff_factory", [ - pytest.param(lambda mols: MMFFBatchedForcefield(mols), id="mmff"), - pytest.param(lambda mols: UFFBatchedForcefield(mols), id="uff"), + pytest.param(MMFFBatchedForcefield, id="mmff"), + pytest.param(UFFBatchedForcefield, id="uff"), ], ) def test_batched_forcefield_metadata_and_element_view(ff_factory): @@ -429,8 +429,8 @@ def test_batched_forcefield_metadata_and_element_view(ff_factory): @pytest.mark.parametrize( "ff_factory", [ - pytest.param(lambda mols: MMFFBatchedForcefield(mols), id="mmff"), - pytest.param(lambda mols: UFFBatchedForcefield(mols), id="uff"), + pytest.param(MMFFBatchedForcefield, id="mmff"), + pytest.param(UFFBatchedForcefield, id="uff"), ], ) def test_batched_forcefield_lazy_build_and_rebuild(ff_factory): @@ -459,8 +459,8 @@ def test_batched_forcefield_lazy_build_and_rebuild(ff_factory): @pytest.mark.parametrize( "ff_factory", [ - pytest.param(lambda mols: MMFFBatchedForcefield(mols), id="mmff"), - pytest.param(lambda mols: UFFBatchedForcefield(mols), id="uff"), + pytest.param(MMFFBatchedForcefield, id="mmff"), + pytest.param(UFFBatchedForcefield, id="uff"), ], ) @pytest.mark.parametrize( diff --git a/nvmolkit/tests/test_skill.py b/nvmolkit/tests/test_skill.py index 00cd8c80..e842fba9 100644 --- a/nvmolkit/tests/test_skill.py +++ b/nvmolkit/tests/test_skill.py @@ -56,6 +56,7 @@ def test_skill_snippet_runs(snippet_idx: int, snippet: str, tmp_path: Path) -> N capture_output=True, text=True, timeout=300, + check=False, ) assert result.returncode == 0, ( f"Skill snippet {snippet_idx} failed:\n" diff --git a/nvmolkit/tests/test_substructure.py b/nvmolkit/tests/test_substructure.py index f04615fa..c76864ec 100644 --- a/nvmolkit/tests/test_substructure.py +++ b/nvmolkit/tests/test_substructure.py @@ -995,10 +995,10 @@ def load_smiles_file(filepath: Path, max_count: int = NUM_SMILES, max_atoms: int mols = [] with open(filepath) as f: for line in f: - line = line.strip() - if not line or line.startswith("#"): + stripped_line = line.strip() + if not stripped_line or stripped_line.startswith("#"): continue - smiles = line.split()[0] if " " in line or "\t" in line else line + smiles = stripped_line.split()[0] if " " in stripped_line or "\t" in stripped_line else stripped_line mol = Chem.MolFromSmiles(smiles) if mol is not None and mol.GetNumAtoms() <= max_atoms: mols.append(mol) @@ -1017,13 +1017,13 @@ def load_smarts_file(filepath: Path) -> tuple[list[Chem.Mol], list[str]]: smarts_strings = [] with open(filepath) as f: for line in f: - line = line.strip() - if not line or line.startswith("#"): + stripped_line = line.strip() + if not stripped_line or stripped_line.startswith("#"): continue - mol = Chem.MolFromSmarts(line) + mol = Chem.MolFromSmarts(stripped_line) if mol is not None: queries.append(mol) - smarts_strings.append(line) + smarts_strings.append(stripped_line) return queries, smarts_strings diff --git a/pyproject.toml b/pyproject.toml index 7d302b9d..31f02664 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -168,6 +168,7 @@ select = [ "PERF", # Performance anti-pattern checks "PIE", # Miscellaneous correctness checks "PLE", # Pylint errors + "PLW", # Pylint warnings "RET", # Return statement checks "RUF", # Ruff-specific checks "SIM", # Simplification checks @@ -176,8 +177,10 @@ select = [ [tool.ruff.lint.per-file-ignores] "__init__.py" = ["D104"] -"nvmolkit/tests/test_*.py" = ["D103"] "benchmarks/*.py" = ["F841", "D103"] +"benchmarks/mcs_bench.py" = ["PLW0603"] +"benchmarks/substruct_bench.py" = ["PLW0603"] +"nvmolkit/tests/test_*.py" = ["D103"] [tool.ruff.lint.pydocstyle] convention = "google" From a610184debf56da35c60d510fc66a3754fdc31cd Mon Sep 17 00:00:00 2001 From: Kevin Boyd Date: Fri, 18 Sep 2026 17:17:43 -0400 Subject: [PATCH 5/5] Localize intentional Ruff suppressions --- benchmarks/etkdg_bench.py | 2 +- benchmarks/mcs_bench.py | 2 +- benchmarks/substruct_bench.py | 2 +- pyproject.toml | 3 --- 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/benchmarks/etkdg_bench.py b/benchmarks/etkdg_bench.py index b0b48151..7457c056 100644 --- a/benchmarks/etkdg_bench.py +++ b/benchmarks/etkdg_bench.py @@ -74,7 +74,7 @@ def _mmff_energies(mol: Chem.Mol) -> list[float | None]: try: ff = AllChem.MMFFGetMoleculeForceField(mol, props, confId=conf.GetId()) energies.append(float(ff.CalcEnergy()) if ff is not None else None) - except Exception: + except Exception: # noqa: PERF203 - isolate failures to the individual conformer energies.append(None) return energies diff --git a/benchmarks/mcs_bench.py b/benchmarks/mcs_bench.py index 26603ef6..23aa04d7 100644 --- a/benchmarks/mcs_bench.py +++ b/benchmarks/mcs_bench.py @@ -179,7 +179,7 @@ def _rdkit_params(config_row: dict) -> rdFMCS.MCSParameters: def _rdkit_worker_init(mol_binaries: list[bytes], params: rdFMCS.MCSParameters) -> None: - global _worker_mols, _worker_params + global _worker_mols, _worker_params # noqa: PLW0603 - process-local worker cache _worker_mols = [Chem.Mol(binary) for binary in mol_binaries] _worker_params = params diff --git a/benchmarks/substruct_bench.py b/benchmarks/substruct_bench.py index 87a4dd3f..d88f36f9 100644 --- a/benchmarks/substruct_bench.py +++ b/benchmarks/substruct_bench.py @@ -99,7 +99,7 @@ def time_it(func: Callable, runs: int = 1, gpu_sync: bool = False) -> tuple[floa def _rdkit_worker_init(query_binaries: list[bytes], max_matches: int): """Initialize worker process with shared query data.""" - global _worker_queries, _worker_params + global _worker_queries, _worker_params # noqa: PLW0603 - process-local worker cache _worker_queries = [Chem.Mol(qb) for qb in query_binaries] _worker_params = Chem.SubstructMatchParameters() _worker_params.uniquify = False diff --git a/pyproject.toml b/pyproject.toml index 31f02664..ea0c052b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -150,7 +150,6 @@ ignore = [ "D100", # Checks for undocumented public module definitions. "E501", # Checks for lines that exceed the specified maximum character length. "E741", # Checks for the use of the characters 'l', 'O', or 'I' as variable names. - "PERF203", # Preserve per-conformer exception isolation in benchmark energy collection. "RET505", # Preserve elif chains after terminating branches. "RUF005", # Checks for uses of the + operator to concatenate collections. ] @@ -178,8 +177,6 @@ select = [ [tool.ruff.lint.per-file-ignores] "__init__.py" = ["D104"] "benchmarks/*.py" = ["F841", "D103"] -"benchmarks/mcs_bench.py" = ["PLW0603"] -"benchmarks/substruct_bench.py" = ["PLW0603"] "nvmolkit/tests/test_*.py" = ["D103"] [tool.ruff.lint.pydocstyle]