add setup.py to install python package #254
Conversation
|
While the package installs correctly, it does not pass the Python tests (in test/serialbox-python/serialbox). These tests also do not pass on our non-pip (normal cmake) install, so it's unclear whether they break due to our installation or the code. In particular, |
|
Hi there, this is jenkins continuous integration... |
|
The tests do now appear to be passing (I replaced a sys.exit with the appropriate set-sentinel-value call), but not on CI. I'm unsure why, but it doesn't appear to be finding CMakeLists.txt at the top level of the repo. You can check it out on our branch tests here. |
|
OK, tests are now passing on CI. The issue was that the older version of pip in the default environment copies the setup.py and other files to a temporary directory away from the source code that needed to compile, so when I tried to grab the sources from two levels up it wasn't finding it. The updated versions now in the build-pip CI step build a wheel with the files in-place. |
|
@havogt Could we kick off CI for this PR? |
|
ok to test |
|
launch jenkins |
There was a problem hiding this comment.
Thanks @mcgibbon! Looks good. I'll ask @egparedes if he wants to have a quick look as he has experience with pip setups.
|
|
||
| DIR = os.path.abspath(os.path.dirname(__file__)) | ||
|
|
||
| # Convert distutils Windows platform specifiers to CMake -A arguments |
There was a problem hiding this comment.
Did you copy this structure from a template and therefore all the windows stuff or does Serialbox actually compile on windows?
There was a problem hiding this comment.
Copied from the cmake setup.py template. No idea whether it does or does not compile on windows. I'd be happy to remove the windows stuff if you can point out what is safe/appropriate to remove, though the only issue it might cause is someone thinking it works on Windows if it does not.
| # logic and declaration, and simpler if you include description/version in a file. | ||
| setup( | ||
| name="serialbox", | ||
| version="0.0.1", |
There was a problem hiding this comment.
We should probably move the version number into a text file (instead of keeping it in the CMakeLists.txt) to make it easily accessible here. I can do that in a separate PR.
There was a problem hiding this comment.
I've had a really good experience using bumpversion to control this, you tell it all the places where your version string exists and it will update them with a command. It keeps track of the version centrally in its own config.
|
launch jenkins |
Co-authored-by: Hannes Vogt <[email protected]>
|
Committed the suggested change, should be ready to go barring the Jenkins test failure being relevant. |
|
Thanks! Could you merge with master, then jenkins should be green as well. |
|
launch jenkins |
This PR adds a setup.py file inside serialbox-python which can be used to pip install it as a python package.
It also adds a
build-pipworkflow step which pip installs serialbox-python and runs the python tests.