nilsnolde / py-osrm

Python bindings to the OSRM routing framework
https://gis-ops.github.io/py-osrm/
BSD 2-Clause "Simplified" License
8 stars 8 forks source link

Add install/build related files #2

Closed whytro closed 1 year ago

whytro commented 1 year ago

Issue

Relevant to:

https://github.com/gis-ops/py-osrm/issues/1 Local build & install of new package

Adds install/build related files so that it can be installed via running python3 -m pip install .

nilsnolde commented 1 year ago

Thanks, I’ll have a look tmrw. It’s a bit of a pity that we have to configure nanobind with CMake (EDIT: or maybe it’s also better, eg win support, let’s see:)). Can you elaborate a bit on the building internals of scikit-build with nanobind (e.g. role of pyproject.toml)?

whytro commented 1 year ago

Sure! As you may be aware, pyproject.toml is the new format to replace setup.py - it takes the role of describing how the wheel should be built (ie. describing build requirements). So in this case, you can see that it describes scikit-build-core and nanobind as dependencies, but also defines a configuration block for scikit-build-core in the form of the [tool.scikit-build] block.

Though the config block included currently is brought over from the nanobind documentation sample:

In the end, the flow simply goes via scikit-build-core looking at the pyproject.toml for the defined config parameters, then running CMake using the CMakeLists file.

I also think that CMake will be of benefit - here I decided to try the approach of only downloading building LibOSRM if it wasn't already installed, using FetchContent instead of git submodules (which would also avoid the issue of users forgetting to initialize the submodules).

nilsnolde commented 1 year ago

Ok great, thanks a lot for elaborating! I'm cool with not having a setup.py and going for a much more modern approach.

SiarheiFedartsou commented 1 year ago

I will take closer look a bit later, but after 5 minutes of looking into code it seems to LGTM. :)