build: improve installation making use of `pyproject.toml` file only and `setuptools` #491

gcroci2 · 2023-09-06T11:50:50Z

I removed redundancies from the actions
Now we have only one config file instead of 3 (pyproject.toml), which contains all the needed info for pip-related dependencies and metadata of deeprank2.
I fixed the builds for multiple python versions; the action.yml file was not taking as input the python version specified in the workflows, thus defaulting always to the one specified (as a default) in the action.yml, which was python 3.9. We were not running workflows on python 3.10.
Now we test the build on Python 3.10 and 3.11; I removed python 3.9 from everywhere, so we're not maintaining it anymore.
I kept the conda installation of msms since ResidueDepth from biopython uses it (tried and verified).
Apparently dependencies' urls (or index urls) are not expected from setuptools: the user should be in charge of choosing the indexes, and not the creator of the package; installing torch, torch_geometric and the other torch adds is not possible using setuptools beacause their installation needs such links (or conda, which is not supported by the toml project file). I've read also here that setuptools does not support such links, and that they don't comply to the new pep specifications. With poetry this was possible, but being not in line with the standards and having chosen to not use poetry for installing the other dependencies of this package, I think we should just instruct the user to install the pytorchs versions suitable for the machine, and we should not be responsible for that. As developers, we'll use actions/install-python-and-package/action.yml as a reference for how to install torch packages.
Same as above for the other external dependencies. Also, is a pain to maintain the instructions update in the readme, so I just left the links to the right installation pages for all the external packages (i.e., packages not installed through the toml file). This makes our readme instructions much more readable, and avoids us to keep updating the readme each time one external dependency changes something (see now, MSMS removed the download file from the website). Eventually, we can update the links for pointing to more up-to-date installation pages, when and if they'll change. Thus, I updated the installation docs.
I removed h5xplorer from everywhere. This PR closes issue Remove h5explorer from package #456 as well.
I tried to setup MacOS installation, and it was running correctly (at least for Python 3.10) except for MSMS installation, which for Mac can't be via conda. I opened an issue about it on their repo, in case they solve it I will readd the MacOS installation. For now I left the OS checking in the action.yml, and in case it will be ready to be run.

DaniBodor

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.

.gitignore

README.md

docs/installation.md

pyproject.toml

README.md

DaniBodor · 2023-09-14T11:48:06Z

README.md

 git clone https://github.com/DeepRank/deeprank2
 cd deeprank2
-pip install -e ./
+pip install -e .
 ```


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]'

Done, but JFYTK you can look at the action.yml file, and there you'll see how the test dependencies are added :)

DaniBodor · 2023-09-14T11:49:38Z

README.md

+  *  [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`


consider adding them to dependencies instead?

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.

gcroci2 · 2023-09-14T14:51:28Z

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.

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 <[email protected]>

README.md

remove setuptools and generate .toml with poetry

d30e96a

gcroci2 self-assigned this Sep 6, 2023

gcroci2 linked an issue Sep 6, 2023 that may be closed by this pull request

Cleaning installation #475

Closed

3 tasks

gcroci2 and others added 27 commits September 9, 2023 21:23

fix include in toml file

874d237

add metadata to project.toml

8fe69e9

add torch to toml file

b9ef344

add poetry to package install action and remove python 3.9

daa031a

remove lock file

890d772

fix torch_geometric additional dependencies installation

08c7e2d

move torch_geometric adds after poetry install

faaf314

place torch installation out of poetry again

ebe0605

run tests with poetry

e7905f6

try to fix the install python and package action

0a68583

print conda packages

a08909f

add print for debugging

486f5eb

try to fix build

8bf697a

remove poetry from installation

c0eac56

add pytest dependency

734d02d

remove lock file

46cc202

add check pytest

fb6d5c0

add check for pytest

ff3323f

remove conda prefix

671486d

fix error in build package yml

6b35457

try to fix build

9041513

try to fit build.yml

6f57188

fix error on build.yml

07e893a

try to fix build.yml

b534d09

add lock file

74e1fd0

add pytest installation

e2a8dac

use only extras in the toml file, remove groups

4834d63

Giulia Crocioni and others added 14 commits September 12, 2023 16:49

update bumpversion.cfg with the toml file

5071ba8

remove unuseg pyg_lib

80c36bb

update installation istructions

f87c7f6

remove remaining ref to h5xplorer

561dae1

update external deps in action.yml

ef2fbe0

add badges for python version and OS

7854637

try to add macos build

be7bcaa

add deps for macos

db98d7e

fix line ending issue

47fa1f1

add pytorch installation for macos

41905a5

try to fix dssp installation for macos

19c186e

add deps for dssp for macos

89d39c4

update instructions for pytables for macos

30fa58c

remove macos build

3a068d6

gcroci2 requested a review from DaniBodor September 13, 2023 16:44

gcroci2 added 2 commits September 13, 2023 18:51

add spaces to the list in the README.md

95d43c9

add spaces to the list in installation.md

1de9e2f

DaniBodor approved these changes Sep 14, 2023

View reviewed changes

gcroci2 changed the title ~~refactor: improve installation and make use of poetry instead of setuptools~~ build: improve installation making use of pyproject.toml file only and setuptools Sep 14, 2023

gcroci2 and others added 5 commits September 14, 2023 16:53

Update README.md

1e3c150

Co-authored-by: Dani Bodor <[email protected]>

Update README.md

f7b64c9

Co-authored-by: Dani Bodor <[email protected]>

Update README.md

d0f21db

Co-authored-by: Dani Bodor <[email protected]>

Update README.md

c072dc6

Co-authored-by: Dani Bodor <[email protected]>

update installation instructions and add empty line to .gitignore

5cfafa0

DaniBodor reviewed Sep 15, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

DaniBodor approved these changes Sep 15, 2023

View reviewed changes

fix codacy errors

77837c8

gcroci2 merged commit fbc545f into main Sep 15, 2023
6 of 7 checks passed

gcroci2 deleted the cleaning_installation_gcroci2 branch September 15, 2023 15:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

gcroci2 commented Sep 6, 2023 •

edited

Loading

DaniBodor left a comment •

edited

Loading

DaniBodor Sep 14, 2023

gcroci2 Sep 14, 2023

DaniBodor Sep 14, 2023

gcroci2 Sep 14, 2023

gcroci2 commented Sep 14, 2023

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

Conversation

gcroci2 commented Sep 6, 2023 • edited Loading

DaniBodor left a comment • edited Loading

Choose a reason for hiding this comment

DaniBodor Sep 14, 2023

Choose a reason for hiding this comment

gcroci2 Sep 14, 2023

Choose a reason for hiding this comment

DaniBodor Sep 14, 2023

Choose a reason for hiding this comment

gcroci2 Sep 14, 2023

Choose a reason for hiding this comment

gcroci2 commented Sep 14, 2023

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

gcroci2 commented Sep 6, 2023 •

edited

Loading

DaniBodor left a comment •

edited

Loading