Skip to content

[ModelicaSystem] reorder input (mypy) #294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 11, 2025

Conversation

syntron
Copy link
Contributor

@syntron syntron commented May 28, 2025

Class ModelicaSystem uses several imputs which are all optional; however, modelName must be present.

This PR reorders the inputs such that modelName is the first parameter. This also fixes one of the last mypy warnings.

Please keep in mind, that this is a breaking change - all calls to ModelicaSystem which do not use kwargs will fail!

On top of PR #292

@syntron syntron mentioned this pull request May 28, 2025
@syntron syntron force-pushed the ModelicaSystem_reorder_input branch 2 times, most recently from 584da4b to 7f8d7e2 Compare June 5, 2025 19:44
Copy link
Member

@adeas31 adeas31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep in mind, that this is a breaking change - all calls to ModelicaSystem which do not use kwargs will fail!

As far as I can see, you updated all the reference calls so we are good to go, right?

@adeas31
Copy link
Member

adeas31 commented Jun 10, 2025

Please rebase the PR.

@syntron syntron force-pushed the ModelicaSystem_reorder_input branch from 7f8d7e2 to 9dbc7b9 Compare June 10, 2025 15:47
syntron added 2 commits June 10, 2025 17:52
* define modelName as first (required!) argument
* use *kwargs in tests
@syntron syntron force-pushed the ModelicaSystem_reorder_input branch from 9dbc7b9 to 9a80503 Compare June 10, 2025 15:53
@syntron
Copy link
Contributor Author

syntron commented Jun 10, 2025

rebased and slightly modifiy such that the order of arguments is still intact for external uses

@adeas31 in my opinion one should always use kwargs (if possible) - it allows to reorder the arguments; however, in this case we can save the existing order (backward compatibility) and also fix the mypy warning by just adding an additional check

@syntron syntron mentioned this pull request Jun 10, 2025
@adeas31 adeas31 merged commit 1edd4e8 into OpenModelica:master Jun 11, 2025
5 checks passed
@syntron syntron deleted the ModelicaSystem_reorder_input branch June 11, 2025 21:03
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