SimVascular / SimVascular/svOneDSolver

buildPy cmake flag does not compile

Open
#120 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.