search

pybind / cmake_example

Example pybind11 module built with a CMake-based build system
Other
616 stars 218 forks source link

chore: cleanup #32

Closed henryiii closed 3 years ago

henryiii commented 3 years ago

Minor touchup based on usage. Dropping the explicit CMake check, as pyproject.toml means that it's rarely missing, and if it is, you get a reasonable error anyway.

YannickJadoul commented 3 years ago

Dropping the explicit CMake check

Should that be added to the setup helpers?

henryiii commented 3 years ago

Should that be added to the setup helpers?

Those help with Setuptools, not CMake. See python_example, not cmake_example for that. Now we could provide a cmake_helpers.py - but at some point, we are just rewriting scikit-build. Personally, I don't think users should patch this together themselves, and should use (and improve if needed) scikit-build instead - that's why I wrote scikit_build_example and we now maintain three examples - setuptools, CMake - hacked together, and scikit-build. 🙄 But this example is popular and people like to feel like they have control, even when it really just means they get to rediscover simple bugs. And I do admit scikit-build's development can be a bit slow at times. (@jcfr :P )

YannickJadoul commented 3 years ago

Those help with Setuptools, not CMake. See python_example, not cmake_example for that.

Sorry, I missed the actual distinction between the two repositories this PR is in!

I guess scikit_build_example is referred to pretty clearly, yes, but it's probably still nice to keep this around, indeed. Even if it's just eductional. (In that case, it never hurts to shout even louder and more frequently "Go look at scikit-build!", ofc.)

jcfr commented 3 years ago

And I do admit scikit-build's development can be a bit slow at times

Agreed 🤷‍♂️ we do plan to spend time going through the PR back log and do a new release. (Ideally before the end of the year)