chainer / chainer-chemistry

Chainer Chemistry: A Library for Deep Learning in Biology and Chemistry

MIT License

629 stars 130 forks source link

refactor general readout: move to links. #305

Closed corochann closed 5 years ago

corochann commented 5 years ago

Could you review? @natsukium

codecov-io commented 5 years ago

Codecov Report

Merging #305 into master will decrease coverage by 6.01%. The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master     #305      +/-   ##
==========================================
- Coverage   89.65%   83.64%   -6.02%     
==========================================
  Files         174      174              
  Lines        7987     7989       +2     
==========================================
- Hits         7161     6682     -479     
- Misses        826     1307     +481

natsukium commented 5 years ago

LGTM but it is better to note down in docstring that GeneralReadout class has no parameter and is interpreted as Link. I think users and other contributors can avoid confusion.

natsukium commented 5 years ago

I think it's good. Thank you!