Rtree Wheel Infrastructure

[mikedh]

Hey, thanks for the great project! I made a pass at building wheels that bundle libspatialindex as it seems like something that trips people up. Happy to close this if it isn't desirable, and thought I'd open it for feedback before I put more time into it.

Changes

• Breaks out library finding logic into separate rtree/finder submodule so that setup.py can reuse it.
• Refactors or replaces wheel building logic with cibuildwheel. I think it's a great project, and I've been able to get it to build every flavor of wheel for other projects.

Status

• without tests enabled this is building wheels successfully on every flavor of Python on Linux.
• I think I'm running into actual issues on some variants of Python that I don't fully understand. The nice thing about cibuildwheel is it runs the tests using the version of the package from the actual wheel and these may be actual issues.

Todo

• Fix tests on Linux platforms.
• Better finder logic for "module path" (it's currently a cheap hack that only works on Linux for testing).
• Get wheels building on MacOS and Windows.

[hobu]

This is awesome. 🎊

• There are flake8 failures now that must be cleaned up.
• Is there a build/test loop of the wheels for each platform? That's what I was trying to do with my scripts and not having much success with. Its great that the wheels are built, but we have no way to know they haven't rotted without a test loop.

[mikedh]

This is awesome. 🎊

Great! I'll try to get this over the finish line then haha. It might be a few weeks, there is potentially a lot of surface area here 😁.

There are flake8 failures now that must be cleaned up.

Absolutely, I'll do a formatting and docstring pass before removing the [WIP] and draft-pr tags and make sure linters pass.

Is there a build/test loop of the wheels for each platform? That's what I was trying to do with my scripts and not having much success with. Its great that the wheels are built, but we have no way to know they haven't rotted without a test loop.

Yeah that is my favorite part: https://cibuildwheel.readthedocs.io/en/stable/options/#test-command
The only thing I'm not totally certain of is how to make sure that it's using the libspatialindex from the wheel, although I guess we could not do a make install.

Any ideas about that cp27m failure? Looks like a dtype thing:

x = array([ 2, 92, 51, 55, 26, 95,  7, 81, 38, 22, 58, 89, 91, 83, 98, 37, 70,
   ...6, 64, 79, 94, 40, 32, 46, 47, 15, 68, 10,  0, 80, 56,
       50], dtype=int64)
y = array([ 2., 92., 51., 55., 26., 95.,  7., 81., 38., 22., 58., 89., 91.,
      ..., 79., 94., 40., 32., 46., 47., 15., 68., 10.,
        0., 80., 56., 50., 30.])
atol = 1e-08, rtol = 1e-05

    def within_tol(x, y, atol, rtol):
        with errstate(invalid='ignore'):
>           return less_equal(abs(x-y), atol + rtol * abs(y))
E           ValueError: operands could not be broadcast together with shapes (69,) (70,)

[hobu]

Any ideas about that cp27m failure? Looks like a dtype thing:

No. We were planning to ditch 2.7 soon anyway.

[hobu]

Making progress?

[mikedh]

Yeah I think this is pretty much ready to go! I've done some manual verification of these wheels here and there, and they seem to work fine. Although there are approximately a bajillion of them, I haven't manually verified all of them.

1. Wheels on Linux/Mac/Windows are building properly and running tests using the wheel against the whole matrix. The shared library search location repair is now done in the CI build scripts, and then we delete build directories and skip make install to make sure tests are being run with the wheel copy of the shared library (system-wide libraries were causing false success at certain points.)
2. I changed finder logic to work with libspatialindex-dev installed system-wide but as a last resort on posix.
3. Conda build looks like it was fixed by adding search locations, but I haven’t tested this manually.
4. To get this to upload wheels to pypi you’ll have to add PYPI_USERNAME and PYPI_PASSWORD through the repo’s secrets interface and remove that if false

Here's the final tally:

Rtree-0.9.5-cp27-cp27m-linux_i686.whl
Rtree-0.9.5-cp27-cp27m-linux_x86_64.whl
Rtree-0.9.5-cp27-cp27m-macosx_10_15_x86_64.whl
Rtree-0.9.5-cp27-cp27mu-linux_i686.whl
Rtree-0.9.5-cp27-cp27mu-linux_x86_64.whl
Rtree-0.9.5-cp27-cp27m-win_amd64.whl
Rtree-0.9.5-cp35-cp35m-linux_i686.whl
Rtree-0.9.5-cp35-cp35m-linux_x86_64.whl
Rtree-0.9.5-cp35-cp35m-macosx_10_15_x86_64.whl
Rtree-0.9.5-cp35-cp35m-win_amd64.whl
Rtree-0.9.5-cp36-cp36m-linux_i686.whl
Rtree-0.9.5-cp36-cp36m-linux_x86_64.whl
Rtree-0.9.5-cp36-cp36m-macosx_10_15_x86_64.whl
Rtree-0.9.5-cp36-cp36m-win_amd64.whl
Rtree-0.9.5-cp37-cp37m-linux_i686.whl
Rtree-0.9.5-cp37-cp37m-linux_x86_64.whl
Rtree-0.9.5-cp37-cp37m-macosx_10_15_x86_64.whl
Rtree-0.9.5-cp37-cp37m-win_amd64.whl
Rtree-0.9.5-cp38-cp38-linux_i686.whl
Rtree-0.9.5-cp38-cp38-linux_x86_64.whl
Rtree-0.9.5-cp38-cp38-macosx_10_15_x86_64.whl
Rtree-0.9.5-cp38-cp38-win_amd64.whl
Rtree-0.9.5-cp39-cp39-linux_i686.whl
Rtree-0.9.5-cp39-cp39-linux_x86_64.whl
Rtree-0.9.5-cp39-cp39-macosx_10_15_x86_64.whl
Rtree-0.9.5-cp39-cp39-win_amd64.whl

[mikedh]

OK, manually verified the wheels from here:
https://github.com/mikedh/rtree/actions/runs/422194320

I made sure there was a fresh install (also check location of rtree.core.rt) and ran a simple 2D index check on the following:

• MacOS Python 3.9
• Windows x86_64 Python 3.8
• Linux x86_64 Python 3.8

This is probably fine to release, although happy to take additional feedback. Also anyone who wants to test the wheels at that link feel free, and if you could post your platform/version that would be great.

[hobu]

This is probably fine to release, although happy to take additional feedback.

Awesome, I'll push it out today or tomorrow

[hobu]

Released. https://pypi.org/project/Rtree/

The GHAs need to be updated to capture the wheels as artifacts and the Publish GHA needs to be updated to push them to PyPI upon release.

[hobu]

An unfortunate failure on Windows Python 3.6 when the Conda package builds. Unclear why it is any different than 3.7 or 3.8, however. https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=253701&view=logs&j=171a126d-c574-5c8c-1269-ff3b989e923d&t=1183ba29-a0b5-5324-8463-2a49ace9e213&l=393

[henryiii]

The GHAs need to be updated to capture the wheels as artifacts and the Publish GHA needs to be updated to push them to PyPI upon release.

This might be helpful?
https://scikit-hep.org/developer/gha_wheels

[mikedh]

Thanks for merging! This will probably generate a whole bunch of issues like #175 just given the volume of platforms haha. I'll try to help address the higher priority/segfault/not-functioning ones when I get a chance.

[hobu]

Do you happen to remember why you were copying in your own CMakeLists.txt file? What is missing in libspatialindex's?