Set up pre-commit (#289)

* Add pre-commit config

* Fix codespell warnings

* Add mypy as pre-commit hook

* Fix remaining mypy errors

* Run pre-commit in github CI

* Run linters before setting up OpenModelica in CI

This should make workflow runs with linter errors fail faster.

* Fix mypy warning on Windows

Author: ondras12345

.github/workflows/Test.yml

@@ -19,16 +19,6 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
-      - name: "Set up OpenModelica Compiler"
-        uses: OpenModelica/[email protected]
-        with:
-          version: ${{ matrix.omc-version }}
-          packages: |
-            omc
-          libraries: |
-            'Modelica 4.0.0'
-      - run: "omc --version"
-
       - name: Set up Python ${{ matrix.python-version }}
         uses: actions/setup-python@v5
         with:
@@ -38,16 +28,25 @@ jobs:
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip
-          pip install . pytest pytest-md pytest-emoji flake8
+          pip install . pytest pytest-md pytest-emoji pre-commit
 
       - name: Set timezone
         uses: szenius/[email protected]
         with:
           timezoneLinux: 'Europe/Berlin'
 
-      - name: Lint with flake8
-        run: |
-          flake8 . --count --statistics
+      - name: Run pre-commit linters
+        run: 'pre-commit run --all-files'
+
+      - name: "Set up OpenModelica Compiler"
+        uses: OpenModelica/[email protected]
+        with:
+          version: ${{ matrix.omc-version }}
+          packages: |
+            omc
+          libraries: |
+            'Modelica 4.0.0'
+      - run: "omc --version"
 
       - name: Run pytest
         uses: pavelzw/pytest-action@v2

.pre-commit-config.yaml

@@ -0,0 +1,39 @@
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v5.0.0
+    hooks:
+      - id: trailing-whitespace
+      - id: end-of-file-fixer
+      - id: check-case-conflict
+      - id: check-docstring-first
+      - id: check-executables-have-shebangs
+      - id: check-shebang-scripts-are-executable
+      - id: mixed-line-ending
+      - id: debug-statements
+      - id: destroyed-symlinks
+      - id: fix-byte-order-marker
+      - id: check-merge-conflict
+      - id: name-tests-test
+        args: [--pytest-test-first]
+
+  - repo: https://github.com/pycqa/flake8
+    rev: '7.2.0'
+    hooks:
+      - id: flake8
+
+  - repo: https://github.com/codespell-project/codespell
+    rev: v2.4.1
+    hooks:
+      - id: codespell
+
+  - repo: https://github.com/pre-commit/mirrors-mypy.git
+    rev: "v1.15.0"
+    hooks:
+      - id: mypy
+        args: []
+        exclude: tests/
+        additional_dependencies:
+          - pyparsing
+          - types-psutil
+          - pyzmq
+          - numpy

OMPython/ModelicaSystem.py

@@ -45,7 +45,7 @@
 import subprocess
 import tempfile
 import textwrap
-from typing import Optional
+from typing import Optional, Any
 import warnings
 import xml.etree.ElementTree as ET
 
@@ -369,20 +369,23 @@ def __init__(
         if modelName is None:
             raise ModelicaSystemError("A modelname must be provided (argument modelName)!")
 
-        self.quantitiesList = []
-        self.paramlist = {}
-        self.inputlist = {}
-        self.outputlist = {}
-        self.continuouslist = {}
-        self.simulateOptions = {}
-        self.overridevariables = {}
-        self.simoptionsoverride = {}
+        self.quantitiesList: list[dict[str, Any]] = []
+        self.paramlist: dict[str, str] = {}  # even numerical values are stored as str
+        self.inputlist: dict[str, list | None] = {}
+        # outputlist values are str before simulate(), but they can be
+        # np.float64 after simulate().
+        self.outputlist: dict[str, Any] = {}
+        # same for continuouslist
+        self.continuouslist: dict[str, Any] = {}
+        self.simulateOptions: dict[str, str] = {}
+        self.overridevariables: dict[str, str] = {}
+        self.simoptionsoverride: dict[str, str] = {}
         self.linearOptions = {'startTime': 0.0, 'stopTime': 1.0, 'stepSize': 0.002, 'tolerance': 1e-8}
         self.optimizeOptions = {'startTime': 0.0, 'stopTime': 1.0, 'numberOfIntervals': 500, 'stepSize': 0.002,
                                 'tolerance': 1e-8}
-        self.linearinputs = []  # linearization input list
-        self.linearoutputs = []  # linearization output list
-        self.linearstates = []  # linearization  states list
+        self.linearinputs: list[str] = []  # linearization input list
+        self.linearoutputs: list[str] = []  # linearization output list
+        self.linearstates: list[str] = []  # linearization states list
 
         if session is not None:
             if not isinstance(session, OMCSessionZMQ):

OMPython/OMCSession.py

@@ -643,7 +643,9 @@ def _getuid() -> int:
         On Windows, volumes are mapped with all files are chmod ugo+rwx,
         so uid does not matter as long as it is not the root user.
         """
-        return 1000 if sys.platform == 'win32' else os.getuid()
+        # mypy complained about os.getuid() not being available on
+        # Windows, hence the type: ignore comment.
+        return 1000 if sys.platform == 'win32' else os.getuid()  # type: ignore
 
     def get_server_address(self) -> Optional[str]:
         if self._dockerNetwork == "separate" and isinstance(self._dockerCid, str):

OMPython/OMParser.py

@@ -468,15 +468,13 @@ def make_elements(strings):
                     skip_start = index + 1
                     if strings[skip_start] == "{":
                         skip_brace += 1
-                        indx = skip_start
-                        while indx < len(strings):
-                            char = strings[indx]
+                        for i in range(skip_start, len(strings)):
+                            char = strings[i]
                             if char == "}":
                                 skip_brace -= 1
                                 if skip_brace == 0:
-                                    index = indx + 1
+                                    index = i + 1
                                     break
-                            indx += 1
 
                 index += 1
 
@@ -523,21 +521,21 @@ def make_elements(strings):
 
 
 def check_for_next_string(next_string):
-    anchorr = 0
-    positionn = 0
-    stopp = 0
+    anchor = 0
+    position = 0
+    stop = 0
 
     # remove braces & keep only the SET's values
-    while positionn < len(next_string):
-        check_str = next_string[positionn]
+    while position < len(next_string):
+        check_str = next_string[position]
         if check_str == "{":
-            anchorr = positionn
+            anchor = position
         elif check_str == "}":
-            stopp = positionn
-            delStr = next_string[anchorr:stopp + 1]
+            stop = position
+            delStr = next_string[anchor:stop + 1]
             next_string = next_string.replace(delStr, '')
-            positionn = -1
-        positionn += 1
+            position = -1
+        position += 1
 
         if isinstance(next_string, str):
             if len(next_string) == 0:
@@ -616,16 +614,14 @@ def skip_all_inner_sets(position):
                                     if brace_count == 0:
                                         break
                                 elif s == "=" and string[position + 1] == "{":
-                                    indx = position + 2
                                     skip_brace = 1
-                                    while indx < end_of_main_set:
-                                        char = string[indx]
+                                    for i in range(position + 2, end_of_main_set):
+                                        char = string[i]
                                         if char == "}":
                                             skip_brace -= 1
                                             if skip_brace == 0:
-                                                position = indx + 1
+                                                position = i + 1
                                                 break
-                                        indx += 1
                                 position += 1
                         position += 1
                 elif char == "{" and string[position + 1] == "{":

OMPython/OMTypedParser.py

@@ -107,9 +107,11 @@ def evaluateExpression(s, loc, toks):
 omcRecord = Forward()
 omcValue = Forward()
 
-TRUE = Keyword("true").setParseAction(replaceWith(True))
-FALSE = Keyword("false").setParseAction(replaceWith(False))
-NONE = (Keyword("NONE") + Suppress("(") + Suppress(")")).setParseAction(replaceWith(None))
+# pyparsing's replace_with (and thus replaceWith) has incorrect type
+# annotation: https://github.com/pyparsing/pyparsing/issues/602
+TRUE = Keyword("true").setParseAction(replaceWith(True))  # type: ignore
+FALSE = Keyword("false").setParseAction(replaceWith(False))  # type: ignore
+NONE = (Keyword("NONE") + Suppress("(") + Suppress(")")).setParseAction(replaceWith(None))  # type: ignore
 SOME = (Suppress(Keyword("SOME")) + Suppress("(") + omcValue + Suppress(")"))
 
 omcString = QuotedString(quoteChar='"', escChar='\\', multiline=True).setParseAction(convertString)

README.md

@@ -53,6 +53,15 @@ online.
   - Submit bugs through the [OpenModelica GitHub issues](https://github.com/OpenModelica/OMPython/issues/new).
   - [Pull requests](https://github.com/OpenModelica/OMPython/pulls) are welcome.
 
+
+## Development
+It is recommended to set up [`pre-commit`](https://pre-commit.com/) to
+automatically run linters:
+```sh
+# cd to the root of the repository
+pre-commit install
+```
+
 ## Contact
 
   - Adeel Asghar, <[email protected]>