AstraZeneca / chemicalx

A PyTorch and TorchDrug based deep learning library for drug pair scoring. (KDD 2022)
https://chemicalx.readthedocs.io
Apache License 2.0
715 stars 87 forks source link

Implement DeepDDI model #63

Closed hzcheney closed 2 years ago

hzcheney commented 2 years ago

Closes #2

Changes

cthoyt commented 2 years ago

Please run tox locally to make sure that all tests are passing before we give a further review

codecov-commenter commented 2 years ago

Codecov Report

Merging #63 (c16f927) into main (5449f96) will increase coverage by 0.06%. The diff coverage is 96.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #63      +/-   ##
==========================================
+ Coverage   93.87%   93.93%   +0.06%     
==========================================
  Files          29       29              
  Lines         832      858      +26     
==========================================
+ Hits          781      806      +25     
- Misses         51       52       +1     
Impacted Files Coverage Δ
chemicalx/models/deepddi.py 95.00% <94.73%> (-5.00%) :arrow_down:
tests/unit/test_models.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5449f96...c16f927. Read the comment docs.

benedekrozemberczki commented 2 years ago

Hi there @hzcheney, do you plan to update this PR?

hzcheney commented 2 years ago

Hi there @hzcheney, do you plan to update this PR?

Yes! I will update this PR ASAP.

benedekrozemberczki commented 2 years ago

The formatting still fails.

benedekrozemberczki commented 2 years ago

These are the errors:

./tests/unit/test_models.py:61:59: BLK100 Black would make changes. ./examples/deepddi_examples.py:22:10: BLK100 Black would make changes. ./examples/epgcnds_examples.py:15:55: BLK100 Black would make changes.

hzcheney commented 2 years ago

I am working on it, it's weird that I run tox locally well.

hzcheney commented 2 years ago

Please help! I have no idea why these errors still happened:

./tests/unit/test_models.py:61:59: BLK100 Black would make changes.
./examples/epgcnds_examples.py:15:55: BLK100 Black would make changes.

Even I did not change the epgcnds_examples.py file. :-)

cthoyt commented 2 years ago

Yesterday, black made a new release that will cause some changes. The options are either:

  1. Wait for a new PR where I make these changes
  2. Pin black (terrible idea! Do not do this!)
  3. Commit the changes black makes when you run tox
hzcheney commented 2 years ago

Yesterday, black made a new release that will cause some changes. The options are either:

  1. Wait for a new PR where I make these changes
  2. Pin black (terrible idea! Do not do this!)
  3. Commit the changes black makes when you run tox

Thank you! It's clear to me now.

benedekrozemberczki commented 2 years ago

Hi there @hzcheney, I suggest updating black on your side. Do you want to finish the PR or we can overtake from you?