search

DeepChainBio / bio-transformers

bio-transformers is a wrapper on top of the ESM/Protbert model, trained on millions on proteins and used to predict embeddings.
https://bio-transformers.readthedocs.io/en/latest/getting_started/install.html
Apache License 2.0
143 stars 31 forks source link

CONTRIBUTING - feedbacks / problems #32

Open martinp7 opened 3 years ago

martinp7 commented 3 years ago

While doing the MR #31 I followed the CONTRIBUTING step-by-step and I have some feedbacks / remarks / problems:

  1. Once the conda environment is installed, the pre-commit was not available (even if the it is available in environment_dev.yaml). I tried to run pip install pre-commit but it did not solve the issue. However, using conda install -c conda-forge pre-commit solved the issue. Maybe we can update the environment_dev.yaml by moving the pre-commit requirement from pip to dependencies (+ add conda-forge channel which is commented for now)
  2. Once the pre-commit is installed, it is advised to launch pre-commit run --all-files. With the latest version of main I had several errors, cf: image
  3. Regarding the pre-commit, isort is mentioned in the CONTRIBUTING.md but it is not defined in the .pre-commit-config.yaml (Remark: mypy is used but not mentioned in the CONTRIBUTING.md)
  4. To ensure the git conventions you can add a hook in the pre-commit by adding the following lines in the .pre-commit-config.yaml`: 
    - repo: https://github.com/alessandrojcm/commitlint-pre-commit-hook
rev: v2.2.0
hooks:
  - id: commitlint
    stages: [commit-msg]
    additional_dependencies: ["@commitlint/config-conventional"]

Some information regarding my laptop: