Skip to content

(I003) update usage of variable_filter#464

Draft
syntron wants to merge 17 commits into
OpenModelica:masterfrom
syntron:I003_filter
Draft

(I003) update usage of variable_filter#464
syntron wants to merge 17 commits into
OpenModelica:masterfrom
syntron:I003_filter

Conversation

@syntron
Copy link
Copy Markdown
Contributor

@syntron syntron commented Apr 23, 2026

update handling of variable_filter

  • use public function ModelicaSystemABC.set_variable_filter() to define

  • process it in ModelicaSystemABC._process_override_data() as command line argument

  • use new processing for variable_filter

  • add unittest test_variable_filter()

@cbu-hw
Copy link
Copy Markdown

cbu-hw commented Apr 24, 2026

I'm not sure what's going on with these masses of huuge PRs (who will review that in finite time?!), if that is an AI running wild or not. On the off chance-that this is some kind of structured stacked-feature way of developing features, have you heard that github now formally supports "stacked pull requests"? https://github.github.com/gh-stack/
Maybe that's something that might be useful for you?

@syntron
Copy link
Copy Markdown
Contributor Author

syntron commented Apr 24, 2026

I'm not sure what's going on with these masses of huuge PRs (who will review that in finite time?!), if that is an AI running wild or not. On the off chance-that this is some kind of structured stacked-feature way of developing features, have you heard that github now formally supports "stacked pull requests"? https://github.github.com/gh-stack/ Maybe that's something that might be useful for you?

I'm the AI running wild ;-) at the end this set of patches was created over a long time. It started quite small (some discussion in PR #404; development started mid of 2025). The small changes accumulated (and new ideas were added) but I never had it down to the point as commit ready steps. However, to get it into OMPython I had to scale it down to handleable pices. But - due to additional requests / discussions - there are new items added at the top ...

The original code is spitted into even smaller commits which show some of the try-and-error development; I reworked it (see: https://github.com/syntron/OMPython/tree/syntron_RFC) and asked if these small commits could be merged into 'steps' of single commit PRs to keep the rebase work low.

The stacked PR idea sounds fine - but is is at the moment not rolled out :-(

syntron added 17 commits May 11, 2026 20:15
[test_*] reorder imports

[tests_ModelicaDoE*] fix pylint hint

* use .items()

[tests_*] use OMSessionABC.get_version()

[test_ModelicaSystemCmd] use get_model_name() instead of access to private variable _model_name

[test_ModelicaSystemOMC] read file using utf-8 encoding / linter fix

[test_ModelicaSystemRunner] update test case

* ModelicaSystemRunner & OMCPath
* ModelicaSystemRunner & OMPathRunnerLocal
* ModelicaSystemRunner & OMPathRunnerBash
* ModelicaSystemRunner & OMPathRunnerBash using docker
* ModelicaSystemRunner & OMPathRunnerBash using WSL (not tested!)

[test_OMCPath] update test case

* OMCPath & OMCSessionZMQ
* OMCPath & OMCSessionLocal
* OMCPath & OMCSessionDocker
* OMCPath & OMCSessionWSL (not tested!)
* OMPathLocal & OMCSessionRunner
* OMPathBash & OMCSessionRunner
* OMPathBash & OMCSessionRunner in docker
* OMPathBash & OMCSessionRunner in WSL (not tested!)

add workflow to run unittests in ./tests

[test_OMParser] use only the public interface => om_parser_basic()

[test_OMTypedParser] rename file / use om_parser_typed()

update tests - do NOT run test_FMIRegression.py

reason:
* it is only a test for OMC / not OMPython specific
* furthermore, it is run automatically via cron job (= FMITest)

[test_ModelExecutionCmd] rename from test_ModelicaSystemCmd
[ModelicaSystemCmd] add missing docstring

[OMCSession] spelling fixes

[OMCSessionCmd] add warning about depreciated class

[OMCSessionABC] remove duplicated code; see OMSessionABC

[OMSessionRunnerABC] define class

[OMCSessionZMQ] call super()__init__()

[OMCPath] fix forward dependency on OMCSessionLocal

[OMSessionException] rename from OMCSessionException

[__init__] fix imports
[README.md] small updates

[__init__] small updates
[pylint] fix 'R1729: Use a generator instead 'all(isinstance(item, tuple) for item in val_evaluated)' (use-a-generator)'

[pylint] fix 'W0237: Parameter 'expr' has been renamed to 'command' in overriding 'OMCSessionZMQ.sendExpression' method (arguments-renamed)'

[pylint] [OM*Path*] fix pylint messags about incompatible definitions
[ModelExecutionException] catch exception if ModelExecutionCmd.run() is used

[bugfix] [ModelicaSystem] fix exception; use ModelicaSystemError (instead of wrong ModelExecutionException)

[bugfix] [ModelicaSystemABC] fix _prepare_input_data() - ensure returned data is dict[str, str]
[compatibility] add class wrapper to provide the depreciation message

[ModelicaSystem] fix / improve wrapper functions for v4.0.0 compatibility

[ModelicaSystemABC] additional checks for setInputs()

[test_ModelicaSystemOMC] add tests for setInputs()

[__init__] define ModelicaSystemDoE at the right point (=> compatibility layer)

[__init__] remove duplicate 'OMCSessionABC' in __all__
[ModelicaSystemABC] remove code for (depreciated) arguments in set*() methods

* define code in the compatibility layer in class ModelicaSystem

[test_ModelicaSystem(OMC)] update tests

* for new version: remove usage of old definition
* for compatibility version: test old definition
[OMCSessionABC] remove execute(); still available in compatibility v4.0.0

[ModelicaSystem] define _set_compatibility_helper() as static

[ModelExecutionCmd] remove depreciated simflags

[test_ModelSystemCmd/ModelExecutionCmd] fix test due to changes

[ModelicaSystemCmd] cleanup - do not define (unused / not useable) class
* use public function `ModelicaSystemABC.set_variable_filter()` to define
* process it in `ModelicaSystemABC._process_override_data()` as command line argument
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants