search

flann-lib / flann

Fast Library for Approximate Nearest Neighbors
http://people.cs.ubc.ca/~mariusm/flann
Other
2.22k stars 647 forks source link

[WIP] Python manylinux wheels #425

Closed Erotemic closed 4 years ago

Erotemic commented 5 years ago

My goal with this PR is to improve FLANN's Python packaging. With modules like pytorch and tensorflow, Python has become the primary language of in-production data science. FLANN is a widely known and easy to use tool --- assuming you can install it. Many applications depend on FLANN, but the opencv bindings aren't recent enough.

This PR tries to perform unnecessary restructure, but it does make a dramatic change: Its elevates the role of Python in the repo by placing a setup.py in the root along with the CMakeLists.txt. I've written the setup.py to use Scikit-Build (skbuild) which cleanly integrates with CMake. Using skbuild makes it easy to build and publish wheels.

Note this does mean that CMake is no longer the primary driver of the Python release. I think this design change makes sense though. Python is a higher level language, so it makes sense that it should use the lower level languages and not vis-versa.

About the WIP status: This current version works well enough for me to build and install wheels. I haven't done full testing, but its basically in working order. However, I had to make changes to functionality of the CMake install. I've been holding off on publishing this because I wanted to ensure it was backwards compatible. I'm not sure how easy or hard that is, but there might be room for discussion about breaking backwards compatibility in this case. Do you really need CMake to build the python module if python builds it?

There is also an issue with the install location of "data" files. Basically scikit-build runs cmake with whatever settings you want based on logic in your setup.py. It then tries to place all of the install targets into the wheel. I've had a hard time trying to get the right wheel package structure. I've hacked it into a semi-workable state, but I could forsee issues where it installs the datafiles in a way that may conflict with other applications. We may also need to discuss this issues with the scikit-build folks.

Erotemic commented 5 years ago

@mariusmuja Are you still maintaining this library? Will this PR have a chance of getting merged?

mariusmuja commented 5 years ago

I'm currently traveling, I won't be near a computer for the next two weeks, I'll have a look at it when I get back.

On September 5, 2019 9:38:32 PM UTC, Jon Crall notifications@github.com wrote:

@mariusmuja Are you still maintaining this library? Will this PR have a chance of getting merged?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/mariusmuja/flann/pull/425#issuecomment-528599648

Erotemic commented 4 years ago

Closed in favor of #432