8000 Fix class_weight parametrization in naive bayes · Issue #1511 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

Fix class_weight parametrization in naive bayes #1511

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.

Sign up for GitHub

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

Jump to bottom
Closed
amueller opened this issue Jan 3, 2013 · 10 comments
Closed

Fix class_weight parametrization in naive bayes #1511

amueller opened this issue Jan 3, 2013 · 10 comments
Labels
Easy Well-defined and straightforward way to resolve Enhancement

Comments

@amueller
Copy link
Member
amueller commented Jan 3, 2013

There are some loose ends after merging #1491 and #1499.
In NB, the class weights are now a lists. They should be a list and also accept an auto keyword. That should be easy enough to do using sklearn.utils.compute_class_weight, see #1491.
@larsmans
Copy link
Member
larsmans commented Jan 3, 2013

The auto variant is quite essential. Without it, NB is not NB. Should we refactor it further to decouple intercept_ from class_weight? Currently, the former is the logarithm of the latter.

@amueller
Copy link
Member Author
amueller commented Jan 3, 2013

Err, sorry I was a bit confused/unclear.
Nothing really changed in NB except the names. But the names should probably have consistent meaning and I don't think the automatic class weights in NB are done by setting class_weight='auto'

@mrshu
Copy link
Contributor
mrshu commented Jan 4, 2013

If you think this might be easy enough for me to do, I'd gladly do it.

@amueller
Copy link
Member Author
amueller commented Jan 4, 2013

It's probably not hard but without really looking into it, I can't say what exactly needs to be done.

@amueller amueller mentioned this issue Jan 6, 2013
Closed
@amueller
Copy link
Member Author

Ok, I have no idea how to actually do this :-/ see my comments in #1525

@mrshu
Copy link
Contributor
mrshu commented Jan 12, 2013

OK, thanks for keeping me posted =)

We can probably close this then.

@amueller
Copy link
Member Author

well, having a somewhat consistent interface would be nice. for example accepting both dicts and lists for class weights in all classifiers.

@amueller
Copy link
Member Author

@mblondel @larsmans @GaelVaroquaux I'm having a bit second thoughts on the renaming of class_priors to class_weight. Do you have any good ideas for a nice interface? Having 'auto' mean opposite things doesn't sound good to me. Also, class_priors='auto' is basically integral part of the NB formulation, while class_weight='auto' is just some heuristic for the LinearModels.

Another thing to consider: The dummy classifier that @mblondel introduced recently used the name class_priors. So if we stick with class_weight in NB, we should probably also rename it in the dummy classifier.

@amueller amueller mentioned this issue Jan 20, 2013
Merged
@rajatkhanduja
Copy link
Contributor

Is this still open?
If it is, I would like to try and help.

@amueller
Copy link
Member Author

@rajatkhanduja the last comment on this is @larsmans saying "let's discuss this after the release", which probably was like 0.13 or 0.14. I'm a bit unsure if there is an issue or not. Maybe it's best to close. But it still feels weird to me.

@amueller amueller closed this as completed Dec 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Easy Well-defined and straightforward way to resolve Enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@larsmans @rajatkhanduja @amueller @mrshu
0