Update mo2fmu.py and its test to run with new Dymola version #11 #12
Conversation
prudhomm
requested review from
Copilot and
prudhomm
and removed request for
vincentchabannes
There was a problem hiding this comment.
Pull Request Overview
Updates the mo2fmu.py module and its test to be compatible with Dymola version 2025xRefresh1, replacing deprecated configuration options and updating file paths.
- Removed invalid
Advanced.Generate32BitBinaryoption and updated 64-bit compilation settings - Changed
Advanced.EnableCodeExportfrom false to true to enable license-free FMU usage - Updated Dymola paths and package references for 2025 version compatibility
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/python/feelpp/mo2fmu/mo2fmu.py | Updated Dymola configuration options and default paths for version 2025 compatibility |
| tests/test_mo2fmu.py | Updated test configuration with new Dymola 2025 paths and package references |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/python/feelpp/mo2fmu/mo2fmu.py
Outdated
| # Turn on full compiler optimizations (instead of the default -O1) :contentReference[oaicite:0]{index=0} | ||
| # **2) Enable code export so FMU contains sources or compiled binaries and no longer requires a license to run ** | ||
| dymola.ExecuteCommand("Advanced.EnableCodeExport=true;") | ||
| # **3) Turn on full compiler optimizations (instead of the default -O1) :contentReference[oaicite:0]{index=0} |
There was a problem hiding this comment.
Removed malformed citation reference ':contentReference[oaicite:0]{index=0}' from comment.
| # **3) Turn on full compiler optimizations (instead of the default -O1) :contentReference[oaicite:0]{index=0} | |
| # **3) Turn on full compiler optimizations (instead of the default -O1)** |
src/python/feelpp/mo2fmu/mo2fmu.py
Outdated
| help='path to Dymola executable.') | ||
| @click.option('--dymolaegg', default="Modelica/Library/python_interface/dymola.egg", type=click.Path(), | ||
| @click.option('--dymolaegg', default="Modelica/Library/python_interface/dymola-2025.1-py3-none-any.whl", type=click.Path(), | ||
| help='path to Dymola egg file, relative to Dymola root.') |
There was a problem hiding this comment.
The help text still refers to 'Dymola egg file' but the default value is now a .whl file. Update the help text to reflect that it can be either an egg or wheel file.
| help='path to Dymola egg file, relative to Dymola root.') | |
| help='path to Dymola egg or wheel file, relative to Dymola root.') |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 14 out of 17 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
pyproject.toml:1
- The default Dymola wheel path specifies version
dymola-2025.1-py3-none-any.whl. Given that your knowledge cutoff is January 2025 and it's currently October 2025, verify this version exists. The PR mentions "dymola-2025xRefresh1" in paths, which suggests this might be a pre-release or specific build number that may not match the standard wheel naming.
[build-system]
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| osString = platform.system() | ||
| isWindows = osString.startswith("Win") | ||
| isWindows = osString.startswith("Win") # noqa: F841 |
There was a problem hiding this comment.
The variable isWindows is assigned but never used (hence the F841 suppression). Consider removing this unused variable or implementing platform-specific logic if it was intended for future use.
| isWindows = osString.startswith("Win") # noqa: F841 |
| dymola_interface.close() | ||
| vdisplay.stop() | ||
| spd.drop('Logger') | ||
| spd.drop("Logger") |
There was a problem hiding this comment.
The logger is being dropped with the hardcoded name "Logger", but the logger was created with a unique name logger_name = f"mo2fmu_{uuid.uuid4().hex[:8]}" on line 57. This mismatch means the logger won't be properly cleaned up. Use spd.drop(logger_name) instead.
2 participants
Work Done
Fixed options that were invalid because they did not exist. The intended purpose behind these invalid options is contained in the other remaining options. (Advanced.Generate32BitBinary doesn't exist and Advanced.CompileWith64 replaces its role by managing both 32-bit and 64-bit compilation.)
Updated repository to run with last Dymola version.
Permanently sets the EnableCodeExport option to true so that FMUs can be used without a license.
Updated the tests.
closes Dymola 2025 Update #11
closes Code Quality Improvements for mo2fmu #13