SimVascular / SimVascular/svOneDSolver
buildPy cmake flag does not compile
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 22
- Forks
- 27
- PR merge metrics
- No merged PRs in 30d
Description
Description
I removed "main.h" since its a-typical and served no obvious purpose. I just found that buildPy required "main.h" but didn't realize that previously since it is not verified in the automated builds. I am suspicious that the buildPy was already defunct before I removed main.h, but that's besides the point.
If there are clients that use buildPy, we should collect their requirements, and refactor to a new header that satisfies those requirements. If there are no clients for it, I recommend removing it. Generally speaking, exposing the internal C++ headers makes the source more rigid, as this failure indicates. We should avoid exposing such details unless there is a use-case that cannot be met through a more standard input argument to the executable.
If we do need to fix it, I recommend we introduce a separate header, e.g., "cvOneDPythonInterface.hpp", rather than "main.h" that exposes limited functionality to the python interface. This would be more obvious than an exposed "main.h". That would help limit our exposure and give us a target for testing the SWIG interface.
@ktbolt @mrp089
Reproduction
Run cmake with buildPy command and then run the make
Expected behavior
Should build, but fails
Additional context
No response
Code of Conduct
- I agree to follow this project's Code of Conduct and Contributing Guidelines
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Run cmake with the buildPy option and then make to reproduce the failure. Inspect main.h, the buildPy configuration, and the SWIG interface, then determine whether clients depend on buildPy. Done means agreeing on removal if it is unused, or defining and testing a limited replacement header interface that builds successfully.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake, cpp, python
- Domain
- api, build-system
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100