fixing lint and adding lint action

.github/workflows/lint.yml

@@ -0,0 +1,17 @@
+name: Lint
+
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+      - uses: pre-commit/[email protected]
+        env:
+          RUFF_OUTPUT_FORMAT: github

.gitignore

@@ -204,4 +204,3 @@ dmypy.json
 
 # Cython debug symbols
 cython_debug/
-

.pre-commit-config.yaml

@@ -0,0 +1,13 @@
+repos:
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.3.1
+    hooks:
+      - id: ruff
+        args:
+          - --fix
+      - id: ruff-format
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.5.0
+    hooks:
+      - id: end-of-file-fixer
+      - id: trailing-whitespace

CMakeLists.txt

@@ -56,4 +56,4 @@ nanobind_add_module(
     # Source code
     src/cpu/lcs_cpu.cpp)
 # target_link_libraries(lcspy PRIVATE lcspy)
-install(TARGETS lcspy_ext LIBRARY DESTINATION lcspy)
+install(TARGETS lcspy_ext LIBRARY DESTINATION lcspy)

pyproject.toml

@@ -74,6 +74,9 @@ testpaths = ["tests"]
 
 [tool.ruff]
 target-version = "py38"
+exclude = ["ext"]
+
+[tool.ruff.lint]
 extend-select = [
     "ARG",
     "A",
@@ -117,6 +120,74 @@ extend-select = [
     "W",
 ]
 
+# Each rule exclusion should be explained here.
+# By default, we think it is better to select groups of rules (above), and exclude
+# specific problematic rules, instead of selecting specific rules. By doing so, in case
+# the ruff rules groups change, this requires us to check and handle the new rules or
+# changes, making sure we stay up to date and keep the best practices.
+
+# ANN003:
+# Would mostly apply to args/kwargs that are passed to methods from dependencies, for
+# which the signature can change depending on the version. This would either be too
+# difficult to comply and/or would add a lot of noqa exceptions. ANN002 is used as it
+# adds very few "noqa" exceptions, but ANN003 would add too much complexity.
+
+# ANN101 and ANN102:
+# Yields errors for `self` in methods from classes, which is unecessary.
+# The existence of these rules is currently questioned, they are likely to be removed.
+# https://github.com/astral-sh/ruff/issues/4396
+
+# B905
+# The `strict` keyword argument for the `zip` built-in method appeared with Python
+# 3.10. As we support previous versions, we cannot comply (yet) with this rule. The
+# exclusion should be removed when dropping support for Python 3.9.
+
+# D107
+# We document classes at the class level (D101). This documentation should cover the
+# way classes are initialized. So we do not document `__init__` methods.
+
+# D203
+# "one-blank-line-before-class", incompatible with D211 (blank-line-before-class).
+# We follow PEP 257 and other conventions by preferring D211 over D203.
+
+# D212
+# "multi-line-summary-first-line", incompatible with D213
+# (multi-line-summary-second-line).
+# We follow PEP 257, which recommend to set put the summary line on the second line
+# after the blank line of the opening quotes.
+
+# FBT001 and FBT002
+# Refactoring all the methods to make boolean arguments keyword only would add
+# complexity and could break code of users. It's ok to have booleans as positional
+# arguments with default values. For code redability though, we enable FB003.
+
+# COM812:
+# Yields errors for one-line portions without comma. Trailing commas are automatically
+# set with ruff format anyway. This exclusion could be removed when this behavior is
+# fixed in ruff.
+
+# UP038
+# Recommends to | type union with `isinstance`, which is only supported since Python
+# 3.10. The exclusion should be removed when dropping support for Python 3.9.
+
+# (ISC001)
+# May cause conflicts when used with the ruff formatter. They recommend to disable it.
+# We leave it enabled but keep this in mind.
+
+ignore = [
+    "ANN003",
+    "ANN101",
+    "ANN102",
+    "B905",
+    "COM812",
+    "D107",
+    "D203",
+    "D212",
+    "FBT001",
+    "FBT002",
+    "UP038",
+]
+
 [tool.ruff.lint.per-file-ignores]
 "tests/**" = [
     "S101",  # allow assertions in tests

src/lcspy/__init__.py

@@ -1,9 +1,5 @@
+"""Main LCSPy module."""
 
-"""from .lcs_bind import (
-    lcs,
-    lcs_length,
-    lcs_table,
-)"""
 from .lcspy_ext import lcs, lcs_length, lcs_table
 
 __version__ = "0.0.1"

src/lcspy/lcs_bind.pyi

@@ -1,14 +1,7 @@
-"""
-
-"""
+"""Python interface."""
 
 from __future__ import annotations
 
-from numpy import ndarray
-
-from . import lcs_ext
-
-
 """def lcs(seq1: ndarray, seq2: ndarray) -> list[int]: ...
 
 

tests/__init__.py

@@ -0,0 +1 @@
+"""Test module."""

tests/lcs_py.py

@@ -1,6 +1,4 @@
-"""
-Python implementation of the lcs algorithm using PyTorch.
-"""
+"""Python implementation of the lcs algorithm using PyTorch."""
 
 from torch import LongTensor, cat
 
@@ -11,7 +9,7 @@ def longest_common_subsequence(x: LongTensor, y: LongTensor) -> LongTensor:
 
     This works with PyTorch tensors.
     """
-    # generate matrix of length of longest common subsequence for subsequences of both sequences
+    # generate matrix for subsequences of both sequences
     lengths = [[0] * (len(y) + 1) for _ in range(len(x) + 1)]
     for i, xi in enumerate(x):
         for j, yi in enumerate(y):
@@ -21,10 +19,10 @@ def longest_common_subsequence(x: LongTensor, y: LongTensor) -> LongTensor:
                 lengths[i + 1][j + 1] = max(lengths[i + 1][j], lengths[i][j + 1])
 
     # read subsequences from the matrix
-    result = []
     j = len(y)
-    for i in range(1, len(x) + 1):
-        if lengths[i][j] != lengths[i - 1][j]:
-            result.append(x[i - 1].reshape(1))
-
+    result = [
+        x[i - 1].reshape(1)
+        for i in range(1, len(x) + 1)
+        if lengths[i][j] != lengths[i - 1][j]
+    ]
     return cat(result).long()

tests/test_lcs_cpu.py

@@ -1,13 +1,10 @@
-"""
-Testing LCS extension on CPU.
-"""
-
-from numpy import all, arange, array
+"""Testing LCS extension on CPU."""
 
 from lcspy import lcs, lcs_length, lcs_table
+from numpy import all, arange, array
 
 
-def test_lcs_simple():
+def test_lcs_simple() -> None:
     r"""
     Test the LCS method with a simple case.
 

tests/test_lcs_py.py

@@ -1,16 +1,12 @@
-"""
-Testing LCS extension on CPU.
-"""
+"""Testing LCS extension on CPU."""
 
-from torch import LongTensor, arange, all
+from torch import LongTensor, all, arange
 
 from .lcs_py import longest_common_subsequence
 
 
-def test_lcs_simple():
-    r"""
-    Tokenize and decode a MIDI back to make sure the possible I/O format are ok.
-    """
+def test_lcs_simple() -> None:
+    r"""Tokenize and decode a MIDI back to make sure the possible I/O format are ok."""
     seq1 = arange(0, 12)
     seq2 = LongTensor([4, 0, 1, 2, 8, 2, 3, 4, 5, 6])
     lcs_ = longest_common_subsequence(seq1, seq2)

tests/utils.py

@@ -1,3 +1,5 @@
+"""Utils for tests."""
+
 from pathlib import Path
 
 HERE = Path(__file__).parent