Skip to content

Remove create_label_dict keyword argument from Learner.train() - #605

Merged
desilinguist merged 2 commits into
masterfrom
565-remove-create-label-dict-argument
May 7, 2020
Merged

Remove create_label_dict keyword argument from Learner.train()#605
desilinguist merged 2 commits into
masterfrom
565-remove-create-label-dict-argument

Conversation

@desilinguist

Copy link
Copy Markdown
Collaborator

This PR closes #565.

  • It also modifies the Learner._create_label_dict() argument so that it internally keeps track of whether label_dict has already been set.
  • No additional tests are necessary since the cross-validation and learning curve tests already represent scenarios where this function is called twice, once in the top-level cross_validate()/learning_curve() methods and once again when the underlying models are trained.

- Modify the `Learner._create_label_dict()` argument so that it internally keeps track of whether `label_dict` has already been set.
@desilinguist
desilinguist requested review from a user, Lguyogiro, aoifecahill and mulhod May 4, 2020 19:49
@codecov

codecov Bot commented May 4, 2020

Copy link
Copy Markdown

Codecov Report

Merging #605 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #605      +/-   ##
==========================================
- Coverage   95.16%   95.16%   -0.01%     
==========================================
  Files          26       26              
  Lines        2978     2977       -1     
==========================================
- Hits         2834     2833       -1     
  Misses        144      144              
Impacted Files Coverage Δ
skll/learner/__init__.py 96.28% <100.00%> (-0.01%) ⬇️

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 547fdd5...0775aca. Read the comment docs.

@aoifecahill

Copy link
Copy Markdown
Collaborator

Looks good to me. What about the drop in code coverage (1 line?). Is that just noise?

@desilinguist

Copy link
Copy Markdown
Collaborator Author

Looks good to me. What about the drop in code coverage (1 line?). Is that just noise?

Yup, we can ignore that.

@mulhod mulhod left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicky typo suggestion. Looks good to me, though!

Comment thread skll/learner/__init__.py Outdated
Co-authored-by: Matt Mulholland <mulhodm@gmail.com>
@desilinguist
desilinguist merged commit a9599ea into master May 7, 2020
@delete-merged-branch
delete-merged-branch Bot deleted the 565-remove-create-label-dict-argument branch May 7, 2020 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove create_label_dict argument from Learner.train()

3 participants