Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Stop advertising support for weights in LogisticRegression. #1448

Merged
merged 2 commits into from
Jul 13, 2016
Merged

[FIX] Stop advertising support for weights in LogisticRegression. #1448

merged 2 commits into from
Jul 13, 2016

Conversation

sstanovnik
Copy link
Contributor

The liblinear solver does not support weights in SKL, even though the parameter is there. Override the property to prevent errors.

The liblinear solver does not support weights in SKL, even though the
parameter is there.
@codecov-io
Copy link

codecov-io commented Jul 12, 2016

Current coverage is 87.91%

Merging #1448 into master will decrease coverage by <.01%

@@             master      #1448   diff @@
==========================================
  Files            77         77          
  Lines          7584       7582     -2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           6668       6666     -2   
  Misses          916        916          
  Partials          0          0          

Sunburst

Powered by Codecov. Last updated by 28b7e58...c5d211b

@lanzagar
Copy link
Contributor

Since codecov is also complaining, you might want to leave a test for logreg (and the new one with linreg replacing the previous one).

assertFalse(LogisticRegressionLearner().supports_weights, ...)
and maybe also assert that learning with weights would raise the exception (so that the override is justified).

@astaric
Copy link
Member

astaric commented Jul 12, 2016

+1 for second assert, this way we will get a failing test if (when) liblinear starts accepting weights.

@lanzagar lanzagar merged commit 7a6c034 into biolab:master Jul 13, 2016
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.

4 participants