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

Add "solver" and "max_iter" parameters to class LogisticRegression #435

Merged
merged 7 commits into from
Mar 25, 2022

Conversation

dSerna4
Copy link
Contributor

@dSerna4 dSerna4 commented Mar 16, 2022

No description provided.

@codecov
Copy link

codecov bot commented Mar 16, 2022

Codecov Report

Merging #435 (b14ae14) into develop (aae3957) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop     #435   +/-   ##
========================================
  Coverage    80.20%   80.20%           
========================================
  Files           93       93           
  Lines         7289     7291    +2     
========================================
+ Hits          5846     5848    +2     
  Misses        1443     1443           
Impacted Files Coverage Δ
skfda/ml/classification/_logistic_regression.py 89.23% <100.00%> (+0.34%) ⬆️

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 aae3957...b14ae14. Read the comment docs.

Copy link
Member

@vnmabus vnmabus left a comment

Choose a reason for hiding this comment

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

Github is showing a full diff because you changed line endings from LF ("\n"), the Unix terminator, to CRLF ("\r\n"), the Windows one. Please try to use just LF as before.

@@ -93,7 +104,7 @@ def fit( # noqa: D102
selected_indexes = np.zeros(self.p, dtype=np.intc)

# multivariate logistic regression
mvlr = mvLogisticRegression(penalty='l2')
mvlr = mvLogisticRegression(penalty='l2', solver=self.solver, max_iter=self.max_iter)
Copy link
Member

Choose a reason for hiding this comment

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

This line is over 79 characters. Split it in multiple lines.

@@ -1,4 +1,5 @@
from __future__ import annotations
import string

Choose a reason for hiding this comment

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

[pep8] reported by reviewdog 🐶
I003 isort expected 1 blank line in imports, found 0

@@ -30,6 +31,12 @@ class LogisticRegression(
p:
number of points (and coefficients) to be selected by
the algorithm.
solver:
Algorithm to use in the multivariate logistic regresion

Choose a reason for hiding this comment

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

[pep8] reported by reviewdog 🐶
W291 trailing whitespace

@@ -93,7 +104,11 @@ def fit( # noqa: D102
selected_indexes = np.zeros(self.p, dtype=np.intc)

# multivariate logistic regression
mvlr = mvLogisticRegression(penalty='l2')
mvlr = mvLogisticRegression(
penalty='l2',

Choose a reason for hiding this comment

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

[pep8] reported by reviewdog 🐶
W291 trailing whitespace

mvlr = mvLogisticRegression(penalty='l2')
mvlr = mvLogisticRegression(
penalty='l2',
solver=self.solver,

Choose a reason for hiding this comment

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

[pep8] reported by reviewdog 🐶
W291 trailing whitespace

@@ -73,9 +80,13 @@ class LogisticRegression(
def __init__(
self,
p: int = 5,
solver: string = 'lbfgs',

Choose a reason for hiding this comment

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

🚫 [mypy] reported by reviewdog 🐶
Module "string" is not valid as a type

@@ -73,9 +79,13 @@ class LogisticRegression(
def __init__(
self,
p: int = 5,
solver: str = 'lbfgs',
Copy link
Member

Choose a reason for hiding this comment

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

This should have a Literal type with the acceptable values.

Copy link
Member

@vnmabus vnmabus left a comment

Choose a reason for hiding this comment

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

LGTM

@vnmabus vnmabus merged commit b8fd7a8 into develop Mar 25, 2022
@vnmabus vnmabus deleted the feature/logisticRegression branch June 28, 2022 14:38
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.

2 participants