10BC0 MRG Class weight refactor by amueller · Pull Request #1464 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

MRG Class weight refactor#1464

Merged
amueller merged 0 commit intoscikit-learn:masterfrom
amueller:class_weight_refactor
Dec 21, 2012
Merged

MRG Class weight refactor#1464
amueller merged 0 commit intoscikit-learn:masterfrom
amueller:class_weight_refactor

Conversation

@amueller
Copy link
Copy Markdown
Member
@amueller amueller commented Dec 11, 2012

Refactor class_weights from SGDClassifier and SVC, which enables the use of classes_ in SVC.

I put it in utils.__init__, since I didn't know a where else to put it. Ideas welcome.

Closes #745, #1037.

@amueller
Copy link
Copy Markdown
Member Author
amueller commented Dec 11, 2012

There is also a labels_ attribute in SVC that I still have to remove. Otherwise this should be good. never mind, think that went away a while ago.

ogrisel
ogrisel reviewed Dec 11, 2012
View reviewed changes
sklearn/utils/__init__.py Outdated
Copy link
Copy Markdown
Member
@ogrisel ogrisel Dec 11, 2012

Choose a reason for hiding this comment

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

This docstring deserves to better describe what's happening for the different values of the class_weight argument.

Copy link
Copy Markdown
Member Author
@amueller amueller Dec 12, 2012

Choose a reason for hiding this comment

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

Good point. Docstring coming tonight.

@amueller
Copy link
Copy Markdown
Member Author
amueller commented Dec 12, 2012

Renamed to MRG. Should be good now.

@amueller
Copy link
Copy Markdown
Member Author
amueller commented Dec 12, 2012

More lines added then deleted :( Well, at least it is consistent with the other estimators now. that has been bothering me for a while.

@amueller amueller mentioned this pull request Dec 15, 2012
Closed
ogrisel
ogrisel reviewed Dec 16, 2012
View reviewed changes
sklearn/svm/tests/test_svm.py Outdated
Copy link
Copy Markdown
Member
@ogrisel ogrisel Dec 16, 2012

Choose a reason for hiding this comment

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

Trailing spaces here.

@ogrisel
Copy link
Copy Markdown
Member
ogrisel commented Dec 16, 2012

I built this branch and the tests pass. +1 for merging once my last comment is addressed.

larsmans
larsmans reviewed Dec 17, 2012
View reviewed changes
sklearn/utils/__init__.py Outdated
Copy link
Copy Markdown
Member
@larsmans larsmans Dec 17, 2012

Choose a reason for hiding this comment

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

This docstring is unclear. I think it should read

Array of class indices per sample; 0 <= y[i] < n_classes for i in range(n_samples).

@amueller
Copy link
Copy Markdown
Member Author
amueller commented Dec 18, 2012

@erg maybe you want to have a look as you complained so much about this ;)

@amueller
Copy link
Copy Markdown
Member Author
amueller commented Dec 18, 2012

Rebased, changed the docstring.

@amueller amueller merged commit 924b178 into scikit-learn:master Dec 21, 2012
@amueller
Copy link
Copy Markdown
Member Author
amueller commented Dec 21, 2012

whoops, messed that one up... merged the wrong branch... hope no-one saw that ;)

@amueller
Copy link
Copy Markdown
Member Author
amueller commented Dec 21, 2012

Ok so this is not merged but I can not reopen it. Great.

@amueller
Copy link
Copy Markdown
< A58B /div>
Member Author
amueller commented Dec 21, 2012

@larsmans @mblondel any more comments? If not, I'll really merge it, otherwise I'll try to create a new PR :-/

That should be a lesson to me: no merging before the first coffee...

@amueller amueller mentioned this pull request Dec 22, 2012
Merged
@amueller amueller mentioned this pull request Jan 3, 2013
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Refactor class_weight logic

4 participants

@amueller @ogrisel @mblondel @larsmans
0