Repository navigation
Conversation
agarny
requested changes
Sep 23, 2026
agarny
left a comment
agarny
left a comment
Contributor
There was a problem hiding this comment.
Looks fine overall, just a few minor requests for changes.
agarny
previously approved these changes
Sep 23, 2026
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
ReferenceRule::UNDEFINED currently converts to an empty string, and several mappings remain untested.
5 open findings
What changed in this PR
Adds string conversion APIs for previously unsupported public enumerations across C++, Python, and JavaScript.
Changes:
- Adds conversions for issue levels/rules, unit prefixes/standard units, and variable interfaces.
- Completes enumeration bindings and mapping data.
- Adds cross-language tests and adjusts coverage detection.
| File | Description |
|---|---|
tests/CMakeLists.txt |
Registers conversion tests. |
tests/as_string/tests.cmake |
Configures the new test target. |
tests/as_string/as_string.cpp |
Tests C++ conversions. |
tests/bindings/python/test_variable.py |
Tests Python variable conversion. |
tests/bindings/python/test_units.py |
Tests Python unit conversions. |
tests/bindings/python/test_issue.py |
Tests Python issue conversions. |
tests/bindings/javascript/variable.test.js |
Tests JavaScript variable conversion. |
tests/bindings/javascript/units.test.js |
Tests JavaScript unit conversions. |
tests/bindings/javascript/issue.test.js |
Tests JavaScript issue conversions. |
src/variable.cpp |
Implements interface conversion. |
src/units.cpp |
Implements unit conversions. |
src/issue.cpp |
Implements issue conversions and repairs mappings. |
src/bindings/javascript/variable.cpp |
Exposes variable conversion. |
src/bindings/javascript/units.cpp |
Exposes unit conversions. |
src/bindings/javascript/issue.cpp |
Completes issue enum bindings and conversions. |
src/bindings/interface/variable.i |
Documents Python variable conversion. |
src/bindings/interface/units.i |
Documents Python unit conversions. |
src/bindings/interface/types.i |
Corrects the issue enum range. |
src/bindings/interface/issue.i |
Documents Python issue conversions. |
src/api/libcellml/variable.h |
Declares variable conversion API. |
src/api/libcellml/units.h |
Declares unit conversion APIs. |
src/api/libcellml/issue.h |
Declares issue conversion APIs. |
cmake/environmentchecks.cmake |
Retries failed Python coverage detection. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…stent with other as_string methods.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Interestingly this picked up a couple of bugs with the enumeration mappings.