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

[MRG] Fix GaussianMixture fit_predict != fit.predict when n_init > 1 #13142

Merged

Conversation

@jeremiedbb
Copy link
Member

commented Feb 12, 2019

Fixes #13070

Move the last _e_step after having set the params to the best one found.
Added a test, which fails on master.

@jeremiedbb

This comment has been minimized.

Copy link
Member Author

commented Feb 12, 2019

Does it go into 0.20.3 or into 0.21 ?

@jnothman
Copy link
Member

left a comment

We would ordinarily release this in 0.21, but I don't know whether we want to make 0.20.3 more extensive as an LTS and hence whether the recent patch to Stratified gold belongs there too.

@jnothman

This comment has been minimized.

Copy link
Member

commented Feb 13, 2019

Please add what's new

@jeremiedbb

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2019

Since the issue was in BaseMixture it also affects BayesianGaussianMixture and indeed the same test fails on master with this estimator. So I added this test in test_bayesian_mixture as well.

@ogrisel ogrisel merged commit 3a47124 into scikit-learn:master Feb 25, 2019

11 checks passed

LGTM analysis: C/C++ No code changes detected
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No alert changes
Details
ci/circleci: deploy Your tests passed on CircleCI!
Details
ci/circleci: doc Your tests passed on CircleCI!
Details
ci/circleci: doc-min-dependencies Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 92.43%)
Details
codecov/project 92.43% (+<.01%) compared to 1c8668b
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ogrisel

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

Merged. Thanks @jeremiedbb!

xhlulu added a commit to xhlulu/scikit-learn that referenced this pull request Apr 28, 2019
xhlulu added a commit to xhlulu/scikit-learn that referenced this pull request Apr 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.