From dcf11f7c280c3325bf1767826d20b1348221a67e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sluka?= Date: Mon, 26 May 2025 21:05:51 +0200 Subject: [PATCH 1/7] Add pre-commit config --- .pre-commit-config.yaml | 27 +++++++++++++++++++++++++++ README.md | 9 +++++++++ 2 files changed, 36 insertions(+) create mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 00000000..9f3424a8 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,27 @@ +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 diff --git a/README.md b/README.md index a477dfe6..2fd6baa1 100644 --- a/README.md +++ b/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, From 5cddad23c5121c1dcf7bd1a1a94e57a83856b81b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sluka?= Date: Mon, 26 May 2025 21:07:14 +0200 Subject: [PATCH 2/7] Fix codespell warnings --- OMPython/OMCSession.py | 2 +- OMPython/OMParser.py | 36 ++++++++++++++++-------------------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/OMPython/OMCSession.py b/OMPython/OMCSession.py index ff1a6cca..494b547b 100644 --- a/OMPython/OMCSession.py +++ b/OMPython/OMCSession.py @@ -582,7 +582,7 @@ def sendExpression(self, command, parsed=True): logger.warning("Result of 'getMessagesStringInternal()' cannot be parsed - set parsed to False!") parsed = False else: - # allways check for error + # always check for error self._omc.send_string('getMessagesStringInternal()', flags=zmq.NOBLOCK) error_raw = self._omc.recv_string() # run error handling only if there is something to check diff --git a/OMPython/OMParser.py b/OMPython/OMParser.py index 7c5fac0e..8b347406 100644 --- a/OMPython/OMParser.py +++ b/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] == "{": From 1f18f675e11c92e62bf11d1f71a6a096dce5a05c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sluka?= Date: Mon, 26 May 2025 21:16:30 +0200 Subject: [PATCH 3/7] Add mypy as pre-commit hook --- .pre-commit-config.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9f3424a8..6611501e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -25,3 +25,14 @@ repos: 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 From 976da2676275ad2ca138f89ee72adea53992f712 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sluka?= Date: Wed, 11 Jun 2025 22:51:52 +0200 Subject: [PATCH 4/7] Fix remaining mypy errors --- .pre-commit-config.yaml | 1 + OMPython/ModelicaSystem.py | 27 +++++++++++++++------------ OMPython/OMTypedParser.py | 8 +++++--- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6611501e..48f9ac64 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -36,3 +36,4 @@ repos: - pyparsing - types-psutil - pyzmq + - numpy diff --git a/OMPython/ModelicaSystem.py b/OMPython/ModelicaSystem.py index c7e382d6..d874104c 100644 --- a/OMPython/ModelicaSystem.py +++ b/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): diff --git a/OMPython/OMTypedParser.py b/OMPython/OMTypedParser.py index 4a585b46..40a345f7 100644 --- a/OMPython/OMTypedParser.py +++ b/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) From d30c133a594109c601f0929d5a601ea6589598fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sluka?= Date: Wed, 11 Jun 2025 23:10:47 +0200 Subject: [PATCH 5/7] Run pre-commit in github CI --- .github/workflows/Test.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/Test.yml b/.github/workflows/Test.yml index 0490b77d..bec35215 100644 --- a/.github/workflows/Test.yml +++ b/.github/workflows/Test.yml @@ -38,16 +38,15 @@ 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/set-timezone@v2.0 with: timezoneLinux: 'Europe/Berlin' - - name: Lint with flake8 - run: | - flake8 . --count --statistics + - name: Run pre-commit linters + run: 'pre-commit run --all-files' - name: Run pytest uses: pavelzw/pytest-action@v2 From 4da2eea180c356e047cae28088baaccf5a25cc19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sluka?= Date: Wed, 11 Jun 2025 23:14:18 +0200 Subject: [PATCH 6/7] Run linters before setting up OpenModelica in CI This should make workflow runs with linter errors fail faster. --- .github/workflows/Test.yml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/Test.yml b/.github/workflows/Test.yml index bec35215..2056b220 100644 --- a/.github/workflows/Test.yml +++ b/.github/workflows/Test.yml @@ -19,16 +19,6 @@ jobs: steps: - uses: actions/checkout@v4 - - name: "Set up OpenModelica Compiler" - uses: OpenModelica/setup-openmodelica@v1.0 - 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: @@ -48,6 +38,16 @@ jobs: - name: Run pre-commit linters run: 'pre-commit run --all-files' + - name: "Set up OpenModelica Compiler" + uses: OpenModelica/setup-openmodelica@v1.0 + with: + version: ${{ matrix.omc-version }} + packages: | + omc + libraries: | + 'Modelica 4.0.0' + - run: "omc --version" + - name: Run pytest uses: pavelzw/pytest-action@v2 with: From 52e86a0b38091c6cef03d512418b9e8657b7151d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sluka?= Date: Wed, 11 Jun 2025 23:22:46 +0200 Subject: [PATCH 7/7] Fix mypy warning For some reason, I only get this warning when running in github actions. --- OMPython/OMCSession.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OMPython/OMCSession.py b/OMPython/OMCSession.py index 494b547b..234fddca 100644 --- a/OMPython/OMCSession.py +++ b/OMPython/OMCSession.py @@ -289,7 +289,6 @@ def __init__(self, self._omc: Optional[Any] = None self._dockerCid: Optional[int] = None self._serverIPAddress = "127.0.0.1" - self._interactivePort = None self._temp_dir = pathlib.Path(tempfile.gettempdir()) # generate a random string for this session self._random_string = uuid.uuid4().hex @@ -437,6 +436,7 @@ def _set_omc_command(self, omc_path_and_args_list) -> list: extraFlags = [] if self._docker: if sys.platform == "win32": + assert self._interactivePort is not None # mypy complained p = int(self._interactivePort) dockerNetworkStr = ["-p", "127.0.0.1:%d:%d" % (p, p)] elif self._dockerNetwork == "host" or self._dockerNetwork is None: