Closed rayegun closed 4 months ago
mmuetzel
commented
4 months ago Looks good to me.
Should the new CMake parameter be named SUITESPARSE_USE_PYTHON (or SUITESPARSE_ENABLE_PYTHON) to better fit the naming convention that is being used for recent changes?
rayegun
commented
4 months ago Good point, updated the naming
mmuetzel
commented
4 months ago It would probably be nice to document the new variable. Maybe, around here? https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/00863cdebe8d2640150ab5c2f087042e6f2858b5/SuiteSparse_config/Config/README.md.in#L982
DrTimothyAldenDavis
commented
4 months ago yes, adding something to the top-level README would be a good idea. Also in the SPEX/README.
I have some other changes to SPEX that are pending, in the paru_dev2 branch. I'm making some major changes to ParU in that branch, and while I was working on that I found some issues with SPEX when building the MATLAB interface. I'll rebase this to the paru_dev2 branch and merge it there.
Then I, or @rayegun, could add the README updates to paru_dev2.
Alternatively, I could do a PR of paru_dev2 into dev2, but I'm hesitant to do that just now without resolving some of the other PRs to dev2 just now.
DrTimothyAldenDavis
commented
4 months ago Correction: I'll go ahead and merge this into dev2 and then keep paru_dev2 updated with PRs from dev2 to paru_dev2. Less churn that way.
Also: I will rename this as SUITESPARSE_USE_PYTHON since the _USE_ style is used for all other options like this one.
DrTimothyAldenDavis
commented
4 months ago
There's currently no way to turn this off when building shared libs, and it would be cleanest to allow turning it off at the SuiteSparse level, not just the SPEX level.