haddocking / HADDOCK-antibody-antigen

Material to run the HADDOCK antibody-antigen modelling protocol
Apache License 2.0
20 stars 9 forks source link

Port code to Python3 #1

Closed antonkulaga closed 2 years ago

antonkulaga commented 2 years ago

I moved the code to python3 and used conda package of ANARCI instead of including it in the repository

Francesco03 commented 2 years ago

Hello, thank you very much for updating this repository.

Just a few comments:

  1. While micromambalooks like a very interesting alternative since most of the user have anaconda installed I would not completely remove it.
  2. What if a user does not want to install anacondaor micromamba? I would keep the instructions to install the dependencies without anaconda or micromamba
  3. In the paper the describes the protocol all the steps are designed according to the current status of the repository. If we are planning to update this code should we think about updating also the paper?
amjjbonvin commented 2 years ago

All very good points indeed!

We should keep the old instruction but also provide the new ones. And I agree that micromamba is not mainstream. Better to provide anaconda instructions.

And in that way the paper is still in line with the repo.

antonkulaga commented 2 years ago

ok, so your suggestions: fallback to conda and keep pip-only instructions, anything else? I actually plan to make a separate cli script that will run everything with one command

amjjbonvin commented 2 years ago

Yes - should be fine.

And we thus will have two sets of instructions, only more manual using pip and one using conda (the easiest way).

Also somewhere we should state the minimal python version

antonkulaga commented 2 years ago

Guys, I made changes according to your suggestions.

I also think that you have to publish the version that was used in the paper as a github release with corresponding stages. In such ways, those users who want to reproduce paper as-is will use that release, while the majority will use later release which is python 3 based. It is 2021, nobody uses python 2 anymore.

amjjbonvin commented 2 years ago

Thanks Anton

And good point - Francesco, I will create a release of the current version before merging this one.

amjjbonvin commented 2 years ago

PS: I also added the Zenodo DOI to the README file, which causes now a conflict. Please update your version

Francesco03 commented 2 years ago

Also the installation without anaconda seems missing to me.

I also want to clarify that by the time I created this repository ANARCI was working only with python2.7. The updated ANARCI code compatible with python3 has been released only ~4 months ago.

antonkulaga commented 2 years ago

@amjjbonvin sorry, I did not notice there was an issue with the git push yesterday and README change was not included. Could you check now? At least in my branch readme is changed https://github.com/antonkulaga/HADDOCK-antibody-antigen

antonkulaga commented 2 years ago

@rvhonorato yes, 3.9 is ok. Please, let me know what is required from me

rvhonorato commented 2 years ago

All good then @antonkulaga, thanks for this edit!

rvhonorato commented 2 years ago

Tests are passing but would be good to run some examples just to double check before we merge this one @Francesco03 @amjjbonvin

Francesco03 commented 2 years ago

Sure. Will do some tests in the coming days and accept the pull request if everything is ok. Thanks a lot!

antonkulaga commented 2 years ago

Sure. Will do some tests in the coming days and accept the pull request if everything is ok. Thanks a lot!

@Francesco03

Sorry, but I added one more commit to PR:

antonkulaga commented 2 years ago

Guys, please let me know what may be needed from my side to have this merged. I plan to add additional PR-s for other features after this one.

Francesco03 commented 2 years ago

Code merged. Thank you very much for your contribution @antonkulaga.