snorkel-team / snorkel

A system for quickly generating training data with weak supervision
https://snorkel.org
Apache License 2.0
5.81k stars 857 forks source link

Refactor BaseLabeler as parent class of label models #1559

Closed vincentschen closed 4 years ago

vincentschen commented 4 years ago

Description of proposed changes

Test plan

Checklist

Need help on these? Just ask!

codecov[bot] commented 4 years ago

Codecov Report

Merging #1559 into master will increase coverage by 0.01%. The diff coverage is 97.56%.

@@            Coverage Diff             @@
##           master    #1559      +/-   ##
==========================================
+ Coverage   97.11%   97.13%   +0.01%     
==========================================
  Files          55       56       +1     
  Lines        2079     2091      +12     
  Branches      342      342              
==========================================
+ Hits         2019     2031      +12     
  Misses         31       31              
  Partials       29       29              
Impacted Files Coverage Δ
snorkel/labeling/model/base_labeler.py 96.96% <96.96%> (ø)
snorkel/labeling/model/baselines.py 100.00% <100.00%> (+2.85%) :arrow_up:
snorkel/labeling/model/label_model.py 95.54% <100.00%> (-0.26%) :arrow_down:
bhancock8 commented 4 years ago

The logic all looks good. The only thing that bugs me is having predict() and score() being strictly pass-through methods on LabelModel. The duplicated passing of arguments and documentation invites staleness. The documentation demonstrating usage should be covered by unit tests anyway. I'd vote we just let the LabelModel inherit those methods from its parent.

bhancock8 commented 4 years ago

Discussed IRL. Decision to leave the methods on LabelModel to keep the Examples section for the documentation.