Skip to content

pip build - #2145

Merged
casperdcl merged 14 commits into
masterfrom
skbuild
Jun 10, 2025
Merged

pip build#2145
casperdcl merged 14 commits into
masterfrom
skbuild

Conversation

@casperdcl

@casperdcl casperdcl commented Apr 23, 2025

Copy link
Copy Markdown
Member

testing

  1. cleanup any old installation (pip uninstall cil ; rm $CONDA_PREFIX/lib/*cilacc.*)
  2. follow the updated README

open follow-up issues

@casperdcl casperdcl self-assigned this Apr 23, 2025
Comment thread src/Core/CMakeLists.txt
Comment thread pyproject.toml
@casperdcl
casperdcl force-pushed the skbuild branch 2 times, most recently from 2907599 to 71bb164 Compare April 23, 2025 13:33

@purepani purepani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are sort of 3 separate tasks that need to be done in some order:

  • switch to nanobind or pybind11 or similar
  • simplify IPP library discovery
  • switch to scikit-build

I was attacking it from the perspective that it would be easier to do 1 and 2 before 3(and I probably unnecessarily mixed up CI changes into there). This PR attacks it by doing 2 and 3 together.
How hard would it be to just simplify IPP library discovery in 1 PR? That seems like the simplest unit to change here, at least to me.

Comment thread .github/workflows/build.yml
Comment thread pyproject.toml
@purepani

purepani commented Apr 23, 2025

Copy link
Copy Markdown
Collaborator

Oh I just realized - are you not doing the build with scikit-build-core yet?
The reason I changed the name for the folder was because I believe scikit-build-core needs the last component of the file name to be the same as the library(otherwise the build doesn't work).

Perhaps making that tiny change in a separate PR in anticipation is a good idea to minimize conflict issues.

Edit: OH I see what's happening; the python package is being built by cmake itself, whereas I just had the c code being built by cmake, which is needed since you're not building a c-extension but a normal shared library

@purepani

Copy link
Copy Markdown
Collaborator

After attempting it myself, it might just be easier to do all 3 at once(at least for me). I don't personally have the cmake knowledge to do the steps separately, and using scikit-build-core makes it way simpler.

@casperdcl

Copy link
Copy Markdown
Member Author

Thanks @purepani.

simplify IPP library discovery in 1 PR

Good point; it's now in #2148

And yes, separate PRs make it easier for review/debugging/approvals.

@github-project-automation github-project-automation Bot moved this to Todo in CIL work Apr 24, 2025
@casperdcl casperdcl moved this from Todo to In Progress in CIL work Apr 24, 2025
@purepani purepani mentioned this pull request Apr 24, 2025
@casperdcl
casperdcl force-pushed the skbuild branch 2 times, most recently from 6c8ece0 to 6f78f05 Compare April 30, 2025 15:10
@casperdcl
casperdcl requested review from gfardell and paskino April 30, 2025 15:18
@casperdcl casperdcl moved this from In Progress to Priority review in CIL work Apr 30, 2025
@casperdcl
casperdcl marked this pull request as ready for review April 30, 2025 15:23
@casperdcl
casperdcl requested a review from a team as a code owner April 30, 2025 15:23
Comment thread src/Core/CMakeLists.txt
@paskino

paskino commented May 8, 2025

Copy link
Copy Markdown
Contributor

currently trying to test conda build but failing, potentially because my conda is old-ish?

@casperdcl casperdcl mentioned this pull request May 9, 2025
9 tasks
Comment thread recipe/bld.bat Outdated

@gfardell gfardell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All seems happy on windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants