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

added description for classes_ in the logistic documentation #12795

Merged
merged 8 commits into from Dec 17, 2018
Merged

added description for classes_ in the logistic documentation #12795

merged 8 commits into from Dec 17, 2018

Conversation

reshamas
Copy link
Member

Referencing PR / Issue

This closes #12219

Note

added classes_ to logistic regression documentation (logistic.py)

cc: @blooraspberry
#wimlds

@reshamas
Copy link
Member Author

reshamas commented Dec 16, 2018

@amueller
One of the checks failed, but I don't know why. I looked at the "details", but don't know the specific error.

@qinhanmin2014
Copy link
Member

Please merge master into your branch.

@adrinjalali
Copy link
Member

You can run flake8 on the file you're changing to see the errors. They're PEP8 issues.

@@ -1178,6 +1178,13 @@ class LogisticRegression(BaseEstimator, LinearClassifierMixin,
Attributes
----------

classes_ : array or shape (n_classes, )
Copy link
Member

Choose a reason for hiding this comment

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

what is array or shape?

Copy link
Member Author

Choose a reason for hiding this comment

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

from looking at other examples, I am guessing it should be:
array, shape (n_classes, )
Is that correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO array, shape ... or list, shape ...

Copy link
Member

Choose a reason for hiding this comment

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

that's acceptable

classes_ : array or shape (n_classes, )
A list of class labels known to the classifier.

In binary classification, one class is usually considered the
Copy link
Member

Choose a reason for hiding this comment

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

Why do you add this paragraph? I don't think we need it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can remove this entire sentence:

   In binary classification, one class is usually considered the
   'positive' class. For example, in labels [-1, 1], 1 is the positive
   class.

@qinhanmin2014
Copy link
Member

Also add it in LogisticRegressionCV?

@@ -1503,6 +1506,9 @@ class LogisticRegressionCV(LogisticRegression, BaseEstimator,

Parameters
----------
classes_ : array, shape (n_classes, )
Copy link
Member

Choose a reason for hiding this comment

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

It's not parameters :)

@@ -1643,6 +1646,8 @@ class LogisticRegressionCV(LogisticRegression, BaseEstimator,

Attributes
----------
classes_ : array, shape (n_classes, )
A list of class labels known to the classifier.
Copy link
Member

Choose a reason for hiding this comment

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

a blank line after it

Copy link
Member

@qinhanmin2014 qinhanmin2014 left a comment

Choose a reason for hiding this comment

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

thanks @reshamas

@qinhanmin2014 qinhanmin2014 merged commit cd8fe16 into scikit-learn:master Dec 17, 2018
amueller pushed a commit to amueller/scikit-learn that referenced this pull request Dec 17, 2018
adrinjalali pushed a commit to adrinjalali/scikit-learn that referenced this pull request Jan 7, 2019
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
koenvandevelde pushed a commit to koenvandevelde/scikit-learn that referenced this pull request Jul 12, 2019
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.

None yet

4 participants