-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow non-conda builds and pip install #70
Conversation
The sad story of a failed install:
I beg to differ |
OK, this suggests that we should be using pyproject.toml https://gitlab.cern.ch/slac_itk/external/pybind11/-/blob/04dd3262f0420d48531a82021fa4895ba14bdcb2/docs/compiling.rst#id2 that is related to some degree to poetry? Something to try next week |
Here's the actual docs https://pybind11.readthedocs.io/en/stable/compiling.html |
pypa/pip#8368 - this is an interesting view into how pip and python installations are such an awful mess |
|
||
PACKAGE_ROOT=$(realpath $HERE/..) | ||
PACKAGE_NAME=pp-sketchlib | ||
PACKAGE_ORG=mrcide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do have a poppunk org on dockerhub, but we could change this. We could also make a bacpop one/ What do you think is best?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docker has changed their licencing in annoying ways, leading to rate limiting on pushes/pulls. We have applied for and got some level of exception for mrcide which is why I put it here. Very happy to use the poppunk one, but we'll need to get our robot user added to that org (there are also organisation size/user limits) and it might end up causing a few issues if we hit rate limits. But we can cross that bridge when we get to it, including getting you to organise an exception for poppunk
@@ -3,4 +3,4 @@ | |||
|
|||
'''PopPUNK sketching functions''' | |||
|
|||
__version__ = '1.7.4' | |||
__version__ = '1.7.6.2' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v2 PR has been pending one final fix (this only changes the CLI options, and will make the python API more robust by making everything support a kwarg, but not change it).
Anyway, I will sort out that merge after this, as that's still some time away from being ready (#67)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, but I will try and include the azure fix in this before merging
This PR adds a non-conda build, proven to work with a docker image which we build on buildkite.
To do this:
Note that I can't work out how to get pybind11 to install before pip tries to install the package. It turns out that if that doesn't happen pip installs a hopelessly broken package with almost no warning! I tried both
install_requires
andsetup_requires
but neither seem to behave. If there's any documentation apart from random SO threads we might be able to find out the correct way of making this work.UPDATE: this pybind11 dependency resolution thing is extremely annoying in practice. There must be some way of declaring a dependency in setup.py that setup.py actually respects? If you know it John, please share. This is not promising (pybind/python_example#20) - my quick read "you can't do this thing, build a wheel instead, enjoy shipping broken source packages"
@johnlees; you will need to update the Azure pipelines to point at the moved repo - they have not run here.