build: improve installation making use of pyproject.toml file only and setuptools#491
build: improve installation making use of pyproject.toml file only and setuptools#491
pyproject.toml file only and setuptools#491Conversation
There was a problem hiding this comment.
Great job, big effort!
PR should be renamed as you are not using poetry in the end. Also, should it be a "build" type rather than a "refactor"?
I could follow the install instructions and could run pytest and prospector without anything getting flagged.
I did notice that somewhere along the way scipy 1.11.1 gets installed, then gets uninstalled and 1.11.2 gets installed. Probably not worthwhile investigating further though.
I left some minor comments here and there.
| git clone https://github.com/DeepRank/deeprank2 | ||
| cd deeprank2 | ||
| pip install -e ./ | ||
| pip install -e . | ||
| ``` |
There was a problem hiding this comment.
it would be nice to also include the command for adding the test packages as well (I always forget the where it needs to go and which part needs to be in quotations): pip install -e '.[test]'
There was a problem hiding this comment.
Done, but JFYTK you can look at the action.yml file, and there you'll see how the test dependencies are added :)
README.md
Outdated
| * [Here](https://ssbio.readthedocs.io/en/latest/instructions/msms.html) for MacOS with M1 chip users | ||
| * [PyTorch](https://pytorch.org/get-started/locally/) | ||
| * We support torch's CPU library as well as CUDA | ||
| * [PyG](https://pytorch-geometric.readthedocs.io/en/latest/install/installation.html) and its optional dependencies: `torch_scatter`, `torch_sparse`, `torch_cluster`, `torch_spline_conv` |
There was a problem hiding this comment.
consider adding them to dependencies instead?
There was a problem hiding this comment.
I remembered why I didn't do it at the end. As you can see here, installing torch via pip requires an --index-url; same for pytorch geometric additional deps: https://pytorch-geometric.readthedocs.io/en/latest/install/installation.html. Could be that we're lucky and pip picks the right things for ubuntu (even it would be weird but still could be), but I'd stick to the official docs which require urls, that can't be provided in the toml file if we're using setuptools. Also in the future it will be easier to add macos build since there pytorch and pytorch geometric can't be installed via pip.
poetry instead of setuptoolspyproject.toml file only and setuptools
I changed the PR name :) regarding scipy, I think it gets installed with torch and then when we install the package via the toml file it gets upgraded because of the requirement there (>= 1.11.2), which is needed. I think it's fine for now |
Co-authored-by: Dani Bodor <d.bodor@esciencecenter.nl>
Co-authored-by: Dani Bodor <d.bodor@esciencecenter.nl>
Co-authored-by: Dani Bodor <d.bodor@esciencecenter.nl>
Co-authored-by: Dani Bodor <d.bodor@esciencecenter.nl>
msmssinceResidueDepthfrom biopython uses it (tried and verified).