Skip to content
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

build: switch to conda dssp installation #549

Merged
merged 63 commits into from
Jan 24, 2024
Merged

Conversation

gcroci2
Copy link
Collaborator

@gcroci2 gcroci2 commented Jan 15, 2024

With this PR, dssp package is installed through conda instead of source or apt-get. This is a usage improvement for the users and is a step forward for deploying deeprank2 on conda (see issue #559). In particular:

  • now the appropriate conda channels (pointing to the right packages' releases) are specified in the action file
  • dssp (version 4.2.2.1) is installed via conda
  • this solves Fix Coveralls coverage 0% #542, by placing an init file into the tests folder. This is solved by ci: implement ruff #554 as well since Ruff automatically added the init file to the tests folder (and each subfolder, even if is not necessary for coveralls to detect all the files). I also added a check in the coveralls workflow file to make the action fail if the coverage drops below 80%.
  • just to be sure since we do not have an action for testing the dockerfile, be sure to run that procedure again

@gcroci2 gcroci2 linked an issue Jan 19, 2024 that may be closed by this pull request
@gcroci2 gcroci2 changed the base branch from main to dev January 22, 2024 09:25
@gcroci2 gcroci2 linked an issue Jan 22, 2024 that may be closed by this pull request
Copy link
Collaborator

@DaniBodor DaniBodor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I tested all 3 installation procedures (docker, env file, manual) and they all work!

I added 2 commits to this branch.

  1. The first one is some automatic formatting updates. I think maybe you don't have the prettier extension installed, so then vs code cannot automatically format the files that use that (most non-python files).
  2. Suggestions to the README.
    I thought it'd be easier to just put everything in a single commit, rather than add a bunch of code suggestions. Feel free to drop either if you don't like it or to adjust as you prefer.

Note that the python 3.11 issue isn't (stably) fixed after all, so please do not close that issue with this PR.

Finally, the newest version of pandas is giving a warning. Maybe we should add Pyarrow to the deeprank dependencies to make it future proof (and avoid the warning).

/tmp/ipykernel_32/1088082951.py:2: DeprecationWarning:
Pyarrow will become a required dependency of pandas in the next major release of pandas (pandas 3.0),
(to allow more performant data types, such as the Arrow string type, and better interoperability with other libraries)
but was not found to be installed on your system.
If this would cause problems for you,
please provide us feedback at pandas-dev/pandas#54466

import pandas as pd

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@gcroci2 gcroci2 removed a link to an issue Jan 24, 2024
@gcroci2
Copy link
Collaborator Author

gcroci2 commented Jan 24, 2024

Looks good! I tested all 3 installation procedures (docker, env file, manual) and they all work!

I added 2 commits to this branch.

  1. The first one is some automatic formatting updates. I think maybe you don't have the prettier extension installed, so then vs code cannot automatically format the files that use that (most non-python files).

Oh right, I installed it now :)

  1. Suggestions to the README.
    I thought it'd be easier to just put everything in a single commit, rather than add a bunch of code suggestions. Feel free to drop either if you don't like it or to adjust as you prefer.

Very nice edits, thanks! I'll leave the commit, I only corrected two typos and edited a couple of subtitles.

Note that the python 3.11 issue isn't (stably) fixed after all, so please do not close that issue with this PR.

I removed it from my initial comment indeed. What if I remove Python 3.11 from the CI for now? In a few weeks we can try to pick up the issue again, check it better, and see if anything has changed. But at least we will publish a "real" stable version that works on Python 3.10. I have added a commit for this, let me know what you think. If you agree, I will also add a comment to issue #540. @DaniBodor

Finally, the newest version of pandas is giving a warning. Maybe we should add Pyarrow to the deeprank dependencies to make it future proof (and avoid the warning).

I think they will add it themselves when installing pandas (as a pandas dependency in their config/toml file). The warning should be to gather feedback on this possibility shortly so that they know if adding this dependency will cause an issue for anyone. But I wouldn't worry about it, it will be installed automatically by pandas when the time comes.

Copy link
Collaborator

@DaniBodor DaniBodor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed an indent level error in the installation.md file (which I probably created when copy/pasting from the README).

docs/installation.md Outdated Show resolved Hide resolved
docs/installation.md Outdated Show resolved Hide resolved
@DaniBodor
Copy link
Collaborator

Looks good! I tested all 3 installation procedures (docker, env file, manual) and they all work!
I added 2 commits to this branch.

  1. The first one is some automatic formatting updates. I think maybe you don't have the prettier extension installed, so then vs code cannot automatically format the files that use that (most non-python files).

Oh right, I installed it now :)

  1. Suggestions to the README.
    I thought it'd be easier to just put everything in a single commit, rather than add a bunch of code suggestions. Feel free to drop either if you don't like it or to adjust as you prefer.

Very nice edits, thanks! I'll leave the commit, I only corrected two typos and edited a couple of subtitles.

Note that the python 3.11 issue isn't (stably) fixed after all, so please do not close that issue with this PR.

I removed it from my initial comment indeed. What if I remove Python 3.11 from the CI for now? In a few weeks we can try to pick up the issue again, check it better, and see if anything has changed. But at least we will publish a "real" stable version that works on Python 3.10. I have added a commit for this, let me know what you think. If you agree, I will also add a comment to issue #540. @DaniBodor

Finally, the newest version of pandas is giving a warning. Maybe we should add Pyarrow to the deeprank dependencies to make it future proof (and avoid the warning).

I think they will add it themselves when installing pandas (as a pandas dependency in their config/toml file). The warning should be to gather feedback on this possibility shortly so that they know if adding this dependency will cause an issue for anyone. But I wouldn't worry about it, it will be installed automatically by pandas when the time comes.

Perfect. I agree with all of this.

gcroci2 and others added 2 commits January 24, 2024 14:42
@gcroci2 gcroci2 merged commit 8718308 into dev Jan 24, 2024
6 checks passed
@gcroci2 gcroci2 deleted the 534_conda_dssp_gcroci2 branch January 24, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants