Refactoring setup.py to enable dry pip install #82
Conversation
| ext_modules = [ | ||
| Extension('dragnet.lcs', | ||
| sources=["dragnet/lcs.pyx"], | ||
| include_dirs=[get_include()], | ||
| sources=["dragnet/lcs.cpp"], |
There was a problem hiding this comment.
How does this work without checking in the generated C++ files?
There was a problem hiding this comment.
I am not sure what you mean. Writing this is in fact unrolling what the cythonize function does under the hood with source names etc.
There was a problem hiding this comment.
Yeah, but I didn't see any call to cythonize so I wasn't sure if it was still generating those files or if those files needed to be checked in to be found.
I've seen other projects just check in the generated files to avoid using cythonize during setup.py. The downside is you have to remember to update those before checking in and it makes the diffs larger.
There was a problem hiding this comment.
Oh. I see now. I did not understand "checking" this way, hence the confusion.
There was a problem hiding this comment.
When I run on a completely clean virtualenv:
pip install git+https://github.com/Yomguithereal/dragnet.git@setup-build-enhancements
The command fails, and I see the following error:
clang: error: no such file or directory: 'dragnet/lcs.cpp'
clang: error: no input files
error: command 'clang' failed with exit status 1
This means that the .cpp files need to be generated before the Extension commands can be called. I'm not opposed to pregenerating the .cpp files, but then we need to ensure they are regenerated after any changes to the .pyx files.
|
I'll confirm this works from a clean pip install using this branch and then merge. |
| ext_modules = [ | ||
| Extension('dragnet.lcs', | ||
| sources=["dragnet/lcs.pyx"], | ||
| include_dirs=[get_include()], | ||
| sources=["dragnet/lcs.cpp"], |
There was a problem hiding this comment.
When I run on a completely clean virtualenv:
pip install git+https://github.com/Yomguithereal/dragnet.git@setup-build-enhancements
The command fails, and I see the following error:
clang: error: no such file or directory: 'dragnet/lcs.cpp'
clang: error: no input files
error: command 'clang' failed with exit status 1
This means that the .cpp files need to be generated before the Extension commands can be called. I'm not opposed to pregenerating the .cpp files, but then we need to ensure they are regenerated after any changes to the .pyx files.
|
Would a commit hook fit the bill? Or would a command to run when releasing a version be better? |
|
I'd prefer a release script than a commit hook. Commit hooks are hard to manage and there's no guarantee that people use them on forks, etc. |
|
Would something of the kind (the |
|
Yes, that looks reasonable, but I want to hold off on merging until the current release is settled since this changes the way the package is built. |
|
Fair enough. Tell me if I can be of any assistance in the future. |
Hello dragnet team. Thanks for your work.
I am submitting this PR as a conversation opener about pip installations. Currently, since the
setup.pyscript relies on having already installedlxml,Cython&numpy, and a lot of persons aren't able to just dopip install dragnetand have it work. See #81 for instance.This PR therefore leverages techniques found on this SO post and ideas from scipy etc. to make a dry
pip install dragnetwork even if the required deps were not installed in an earlier command.It works on my end on a macOS python 3.6.5, but unfortunately I can't easily test this for further cases. So I was wondering what you think of it and if it could be of interest to you that we try and implement those workarounds or if just a patch to the documentation about pip installs would be enough.
Have a good day