Add testing and linting workflows

Author: pganguli

.github/workflows/pytest_pylint.yml

@@ -0,0 +1,41 @@
+name: Pytest and Pylint
+
+on:
+  push:
+    branches: [master]
+  pull_request:
+    branches: [master]
+
+jobs:
+  test-and-lint:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: ["3.12"]
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v6
+
+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v6
+        with:
+          python-version: ${{ matrix.python-version }}
+
+      - name: Cache pip
+        uses: actions/cache@v5
+        with:
+          path: ~/.cache/pip
+          key: ${{ runner.os }}-pip-${{ hashFiles('**/pyproject.toml') }}
+          restore-keys: ${{ runner.os }}-pip-
+
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install .
+          pip install pytest pylint
+
+      - name: Run pytest
+        run: python -m pytest -q
+
+      - name: Run pylint
+        run: pylint $(git ls-files '*.py')

main.py

@@ -16,6 +16,7 @@
 
 
 def main() -> None:
+    """Demo script: create plant, design controller, simulate and save plot."""
     plant = InvertedPendulum()
     plant = add_delay(plant, T_input=0.5, T_output=0.5)
     plant = discretize(plant, dt=0.01)

pyproject.toml

@@ -14,3 +14,7 @@ dependencies = ["control>=0.9", "numpy>=1.24", "matplotlib>=3.7"]
 
 [tool.setuptools.packages.find]
 where = ["src"]
+
+[tool.pytest.ini_options]
+testpaths = ["tests"]
+

src/control_toolbox/controller.py

@@ -15,6 +15,7 @@
     Nbar    — precompensator              (n_inputs x n_tracked)
               None if closed-loop DC gain is singular (regulates to zero only)
 """
+# pylint: disable=invalid-name
 
 import warnings
 from dataclasses import dataclass
@@ -40,40 +41,18 @@ class ControllerResult:
     tracked: dict[str, float]
 
 
-def build_controller(
-    plant: StateSpace,
-    track: dict[str | int, float | None] | None = None,
-    p: float = 1.0,
-    Q: np.ndarray | None = None,
-    R: np.ndarray | None = None,
-) -> ControllerResult:
-    """LQR controller for a StateSpace plant.  Supports both continuous-
-    and discrete-time systems (uses ``lqr`` or ``dlqr`` depending on
-    ``plant.dt``). The returned closed-loop model preserves the sampling
-    period so that downstream simulation routines behave correctly.
+def _parse_tracking(
+    plant: StateSpace, track: dict | None
+) -> tuple[list[str], dict[str, float], np.ndarray]:
+    """Interpret the ``track`` argument.
 
-    Args:
-        plant:  Any StateSpace instance from systems.py.
-        track:  Dict of {output_name_or_index: target_value | None}.
-                None value = don't care. Omitted outputs = don't care.
-                track=None regulates all outputs to zero.
-        p:      Output-error weight (Q = p * C_track' @ C_track). Ignored if Q given.
-        Q:      State-cost matrix  (n_states x n_states). Overrides p.
-        R:      Input-cost matrix  (n_inputs x n_inputs). Defaults to identity.
-
-    Returns:
-        ControllerResult containing ``K``, ``Nbar`` and ``sys_cl`` (plus
-        the ``tracked`` dictionary used to compute the controller).  If
-        ``plant`` was discrete-time, the returned ``sys_cl`` has its
-        ``dt`` field set accordingly and ``K`` is computed via ``dlqr``.  The
-        precompensator ``Nbar`` is computed with the appropriate steady-
-        state formula for continuous or discrete dynamics.
+    Returns ``(labels, tracked, C_track)`` where ``labels`` is the list of
+    plant output names, ``tracked`` maps those names to non-None reference
+    values, and ``C_track`` is the corresponding rows of the plant output
+    matrix.  This isolates a bulky dictionary comprehension from
+    ``build_controller``.
     """
-    A = np.array(plant.A, dtype=float)
-    B = np.array(plant.B, dtype=float)
     C = np.array(plant.C, dtype=float)
-    n_in = B.shape[1]
-
     labels = (
         list(plant.output_labels)
         if plant.output_labels
@@ -87,47 +66,67 @@ def build_controller(
     }
     idx = [labels.index(lbl) for lbl in tracked]
     C_track = C[idx, :]
+    return labels, tracked, C_track
 
-    # LQR / DLQR depending on plant type
-    Q = p * C_track.T @ C_track if Q is None else np.array(Q, dtype=float)
-    R = np.eye(n_in) if R is None else np.array(R, dtype=float)
-    is_discrete = plant.dt not in (0, None)
-    if is_discrete:
-        K, _, _ = dlqr(A, B, Q, R)
-    else:
-        K, _, _ = lqr(A, B, Q, R)
 
-    # Nbar: solves C_track @ DC-gain(A_cl,B) @ Nbar = -I
-    # For continuous DC gain = -C_track @ inv(A_cl) @ B
-    # For discrete DC gain = C_track @ inv(I - A_cl) @ B
+def _compute_Nbar(A, B, C_track, K, is_discrete):
+    """Calculate the precompensator Nbar from closed-loop data."""
     A_cl = A - B @ K
     if is_discrete:
-        # discrete-time steady-state: x = inv(I - A_cl) B Nbar r
-        # require C_track @ inv(I - A_cl) B Nbar = I
-        # use pseudo-inverse in case (I-A_cl) is singular
         M = C_track @ np.linalg.pinv(np.eye(A_cl.shape[0]) - A_cl) @ B
     else:
-        # continuous-time steady-state: x = -inv(A_cl) B Nbar r
-        # require -C_track @ inv(A_cl) B Nbar = I
-        # use pseudo-inverse in case A_cl is singular
         M = -C_track @ np.linalg.pinv(A_cl) @ B
-    Nbar = np.linalg.pinv(M) if np.all(np.isfinite(M)) else None
-    if Nbar is not None and len(idx) != n_in:
+    return np.linalg.pinv(M) if np.all(np.isfinite(M)) else None
+
+
+def build_controller(
+    plant: StateSpace,
+    track: dict[str | int, float | None] | None = None,
+    p: float = 1.0,
+    Q: np.ndarray | None = None,
+    R: np.ndarray | None = None,
+) -> ControllerResult:
+    """LQR controller for a StateSpace plant.  Supports both continuous-
+    and discrete-time systems (uses ``lqr`` or ``dlqr`` depending on
+    ``plant.dt``). The returned closed-loop model preserves the sampling
+    period so that downstream simulation routines behave correctly.
+    """
+    A = np.array(plant.A, dtype=float)
+    B = np.array(plant.B, dtype=float)
+
+    labels, tracked, C_track = _parse_tracking(plant, track)
+
+    # LQR / DLQR depending on plant type; inline Q and R defaults
+    if plant.dt not in (0, None):
+        K, _, _ = dlqr(
+            A,
+            B,
+            p * C_track.T @ C_track if Q is None else np.array(Q, dtype=float),
+            np.eye(B.shape[1]) if R is None else np.array(R, dtype=float),
+        )
+        discrete = True
+    else:
+        K, _, _ = lqr(
+            A,
+            B,
+            p * C_track.T @ C_track if Q is None else np.array(Q, dtype=float),
+            np.eye(B.shape[1]) if R is None else np.array(R, dtype=float),
+        )
+        discrete = False
+
+    Nbar = _compute_Nbar(A, B, C_track, K, discrete)
+    if Nbar is not None and len(tracked) != B.shape[1]:
         warnings.warn(
             "n_tracked != n_inputs: Nbar is a pseudoinverse; "
             "exact tracking not guaranteed.",
             stacklevel=2,
         )
 
-    n_tracked = len(idx)
-    B_cl = B @ (Nbar if Nbar is not None else np.zeros((n_in, n_tracked)))
-    D_cl = np.zeros((C.shape[0], n_tracked))
-    # preserve sampling period if discrete
     sys_cl = ss(
-        A_cl,
-        B_cl,
-        C,
-        D_cl,
+        A - B @ K,
+        B @ (Nbar if Nbar is not None else np.zeros((B.shape[1], len(tracked)))),
+        np.array(plant.C, dtype=float),
+        np.zeros((np.array(plant.C, dtype=float).shape[0], len(tracked))),
         inputs=[f"r_{lbl}" for lbl in tracked],
         outputs=labels,
         dt=plant.dt,

src/control_toolbox/metrics.py

@@ -63,6 +63,7 @@ def compute_metrics(resp: SimulationResult, settling_band: float = 0.02) -> Metr
         - If the reference is zero the percentage metrics are computed
           against the absolute deviation instead of a percentage.
     """
+    # pylint: disable=too-many-locals
     overshoot_d = {}
     settling_time_d = {}
     steady_state_error_d = {}

src/control_toolbox/plot.py

@@ -19,6 +19,65 @@
 _INPUT_COLOURS = ["#16a34a", "#65a30d", "#0d9488", "#ca8a04"]
 
 
+def _annotate_metrics(ax, t, y, ref, metrics, colour):  # pylint: disable=too-many-arguments,too-many-positional-arguments
+    """Add settling band, time and overshoot annotations to a single axis."""
+    band = 0.02 * (abs(ref) if abs(ref) > 1e-12 else 1.0)
+    ax.axhspan(ref - band, ref + band, alpha=0.08, color=colour, label="±2% band")
+
+    st = metrics.settling_time.get(ref)
+    if st is not None:
+        ax.axvline(st, color=colour, lw=1, ls=":", alpha=0.7)
+        ax.annotate(
+            f"  t_s={st:.2f}s",
+            xy=(st, ref),
+            fontsize=8,
+            color=colour,
+            va="center",
+        )
+
+    os_pct = metrics.overshoot.get(ref, 0.0)
+    if os_pct > 0.1:
+        peak_idx = np.argmax(y) if ref >= 0 else np.argmin(y)
+        ax.annotate(
+            f"OS={os_pct:.1f}%",
+            xy=(t[peak_idx], y[peak_idx]),
+            xytext=(0, 8),
+            textcoords="offset points",
+            fontsize=8,
+            color=colour,
+            ha="center",
+        )
+
+
+def _plot_output(ax, resp, i, lbl, metrics):
+    """Draw a single output subplot including reference/metrics."""
+    colour = _OUTPUT_COLOURS[i % len(_OUTPUT_COLOURS)]
+    y = resp.y[i]
+    ax.plot(resp.t, y, color=colour, lw=2, label=lbl)
+    if lbl in resp.tracked:
+        ref = resp.tracked[lbl]
+        ax.axhline(
+            ref, color=colour, lw=1, ls="--", alpha=0.5, label=f"reference ({ref})"
+        )
+        if metrics is not None:
+            _annotate_metrics(ax, resp.t, y, ref, metrics, colour)
+    ax.set_ylabel(lbl)
+    ax.legend(loc="upper right", fontsize=8)
+    ax.grid(True, alpha=0.3)
+    ax.yaxis.set_major_formatter(ticker.FormatStrFormatter("%.3g"))
+
+
+def _plot_input(ax, resp, j, lbl):
+    """Draw a single input subplot."""
+    colour = _INPUT_COLOURS[j % len(_INPUT_COLOURS)]
+    ax.plot(resp.t, resp.u[j], color=colour, lw=2, label=f"u: {lbl}")
+    ax.axhline(0, color="k", lw=0.5, alpha=0.3)
+    ax.set_ylabel(f"u ({lbl})")
+    ax.legend(loc="upper right", fontsize=8)
+    ax.grid(True, alpha=0.3)
+    ax.yaxis.set_major_formatter(ticker.FormatStrFormatter("%.3g"))
+
+
 def plot_response(
     resp: SimulationResult,
     metrics: Metrics | None = None,
@@ -58,65 +117,11 @@ def plot_response(
         title = "Closed-loop response:  " + ",   ".join(parts)
     fig.suptitle(title, fontsize=12)
 
-    # --- Output subplots -----------------------------------------------------
     for i, lbl in enumerate(resp.output_labels):
-        ax = axes[i]
-        colour = _OUTPUT_COLOURS[i % len(_OUTPUT_COLOURS)]
-        y = resp.y[i]
-
-        ax.plot(resp.t, y, color=colour, lw=2, label=lbl)
-
-        if lbl in resp.tracked:
-            ref = resp.tracked[lbl]
-            ax.axhline(
-                ref, color=colour, lw=1, ls="--", alpha=0.5, label=f"reference ({ref})"
-            )
-
-            if metrics is not None:
-                band = 0.02 * (abs(ref) if abs(ref) > 1e-12 else 1.0)
-                ax.axhspan(
-                    ref - band, ref + band, alpha=0.08, color=colour, label="±2% band"
-                )
-
-                st = metrics.settling_time.get(lbl)
-                if st is not None:
-                    ax.axvline(st, color=colour, lw=1, ls=":", alpha=0.7)
-                    ax.annotate(
-                        f"  t_s={st:.2f}s",
-                        xy=(st, ref),
-                        fontsize=8,
-                        color=colour,
-                        va="center",
-                    )
-
-                os_pct = metrics.overshoot.get(lbl, 0.0)
-                if os_pct > 0.1:
-                    peak_idx = np.argmax(y) if ref >= 0 else np.argmin(y)
-                    ax.annotate(
-                        f"OS={os_pct:.1f}%",
-                        xy=(resp.t[peak_idx], y[peak_idx]),
-                        xytext=(0, 8),
-                        textcoords="offset points",
-                        fontsize=8,
-                        color=colour,
-                        ha="center",
-                    )
-
-        ax.set_ylabel(lbl)
-        ax.legend(loc="upper right", fontsize=8)
-        ax.grid(True, alpha=0.3)
-        ax.yaxis.set_major_formatter(ticker.FormatStrFormatter("%.3g"))
-
-    # --- Input subplots ------------------------------------------------------
+        _plot_output(axes[i], resp, i, lbl, metrics)
+
     for j, lbl in enumerate(resp.input_labels):
-        ax = axes[n_out + j]
-        colour = _INPUT_COLOURS[j % len(_INPUT_COLOURS)]
-        ax.plot(resp.t, resp.u[j], color=colour, lw=2, label=f"u: {lbl}")
-        ax.axhline(0, color="k", lw=0.5, alpha=0.3)
-        ax.set_ylabel(f"u ({lbl})")
-        ax.legend(loc="upper right", fontsize=8)
-        ax.grid(True, alpha=0.3)
-        ax.yaxis.set_major_formatter(ticker.FormatStrFormatter("%.3g"))
+        _plot_input(axes[n_out + j], resp, j, lbl)
 
     axes[-1].set_xlabel("Time  (s)")
     plt.tight_layout()

src/control_toolbox/preprocess.py

@@ -10,6 +10,7 @@
     ctrl = build_controller(plant, ...)
     K, Nbar, sys_cl = ctrl.K, ctrl.Nbar, ctrl.sys_cl
 """
+# pylint: disable=invalid-name
 
 import numpy as np
 from control import StateSpace, c2d, pade, series, ss, tf