Trying pyproject.toml and scikit-build-core

ajfriend commented 4 months ago

Trying for a PEP 621 compatible setup using pyproject.toml instead of setup.py. Also trying to use scikit-build-core.

ajfriend commented 4 months ago

Currently running into an issue:

  CMake Error at src/h3/_cy/CMakeLists.txt:1 (find_package):
    No "FindCython.cmake" found in CMAKE_MODULE_PATH.

  CMake Warning (dev) at src/h3/_cy/CMakeLists.txt:1 (find_package):
    FindCython.cmake must either be part of this project itself, in this case
    adjust CMAKE_MODULE_PATH so that it points to the correct location inside
    its source tree.

    Or it must be installed by a package which has already been found via
    find_package().  In this case make sure that package has indeed been found
    and adjust CMAKE_MODULE_PATH to contain the location where that package has
    installed FindCython.cmake.  This must be a location provided by that
    package.  This error in general means that the buildsystem of this project
    is relying on a Find-module without ensuring that it is actually available.

Based on the comment "You’ll need to handle the generation of files by Cython directly at the moment. A helper (similar to scikit-build classic) might be added in the future." in https://scikit-build-core.readthedocs.io/en/latest/getting_started.html#cmake-file, I'm wondering if this is something from scikit-build classic that we need to remove and just do manually?

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.33%. Comparing base (cec946a) to head (121314a). Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
tests/test_cython/test_cython.py	50.00%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #378 +/- ## =========================================== - Coverage 100.00% 99.33% -0.67% =========================================== Files 10 28 +18 Lines 343 1346 +1003 =========================================== + Hits 343 1337 +994 - Misses 0 9 +9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ajfriend commented 3 months ago

Tests are running fine, but we need to figure out how to build the annotations again: https://github.com/scikit-build/cython-cmake/blob/main/src/cython_cmake/cmake/UseCython.cmake#L201-L202

I've tried just dropping in an --annotate in the CMakeLists.txt but that doesn't seem to be working.

ajfriend commented 3 months ago

TODO: We were previously excluding unnecessary files in the sdist with MANIFEST.in. Those files are now being included, so we need to recover that functionality with this setup.

ajfriend commented 3 months ago

Coverage decreased because I added the test files themselves to be included in the coverage report.

ajfriend commented 3 months ago

Coverage decreased because I added the test files themselves to be included in the coverage report.

And a good thing too, because it caught a bug!

Our Cython example is no longer working:

❯ env/bin/cythonize -i tests/test_cython/cython_example.pyx
/Users/aj/work/h3-py/tests/test_cython/cython_example.c:22015:13: warning: code will never be executed [-Wunreachable-code]
            goto bad;
            ^~~~~~~~
1 warning generated.
ld: warning: duplicate -rpath '/Users/aj/.pyenv/versions/3.12.3/lib' ignored
ld: warning: duplicate -rpath '/opt/homebrew/lib' ignored
error: could not create 'src/tests/test_cython/cython_example.cpython-312-darwin.so': No such file or directory

ajfriend commented 4 weeks ago

Everything should work, except for https://github.com/uber/h3-py/issues/396

I think we should merge this change to master and fix the issue from there.

uber / h3-py

Trying pyproject.toml and scikit-build-core #378

Codecov Report