Skip to content

[MRG] MAINT sync/add support sample_weight with liblinear #15038

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

Merged

Conversation

glemaitre
Copy link
Member

Add support for sample_weight for the loss/penalty in liblinear.

Hopefully it will close:
closes #10873
closes #15018

@glemaitre glemaitre changed the title [WIP] MAINT sync/add support sample_weight with liblinear [MRG] MAINT sync/add support sample_weight with liblinear Sep 20, 2019
@glemaitre
Copy link
Member Author

@amueller you might want to have a look at that

Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

Not a complete review, but looks pretty good.

@adrinjalali adrinjalali self-assigned this Oct 4, 2019
@glemaitre
Copy link
Member Author

@adrinjalali any other comments?

Copy link
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

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

I can confirm the tests fail in master...

Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

nits, otherwise, as far as I can tell, this looks good.

Copy link
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

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

I've checked the C code looks reasonably safe.... and that the tests are valid... I'd do better to compare to upstream, but I'm okay to see this merged.

@adrinjalali
Copy link
Member

Just realized this kinda needs a whats_new entry.

@NicolasHug NicolasHug merged commit 2b60d32 into scikit-learn:master Oct 25, 2019
@NicolasHug
Copy link
Member

Thanks @glemaitre !

@amueller
Copy link
Member

Sweet!

@csbrown
Copy link

csbrown commented Dec 2, 2019

Is this functionality in any of the stable sklearn releases? What should I upgrade to?

@NicolasHug
Copy link
Member

Not yet. The new release should be out this week

@csbrown
Copy link

csbrown commented Dec 2, 2019

Also, would anyone be keen on updating the old docs to mention the fact that sample_weight does nothing at all?

@glemaitre
Copy link
Member Author

Also, would anyone be keen on updating the old docs to mention the fact that sample_weight does nothing at all?

Since this is a bug, it would be better to backport the fix. But we will probably release tomorrow, not sure that it would be worth.

sambhav added a commit to sambhav/scikit-learn that referenced this pull request Dec 12, 2019
With the latest release of sklearn, we have merged scikit-learn#15038
which officially adds support for sample weights to these models. Let's update the docs to reflect this.
sambhav added a commit to sambhav/scikit-learn that referenced this pull request Dec 12, 2019
With the latest release of sklearn, we have merged scikit-learn#15038
which officially adds support for sample weights to these models. Let's update the docs to reflect this.
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.

LinearSVC ignores sample weights
6 participants