Skip to content

[MRG+2] Correct Deprecation of DPGMM and VBGMM.#7124

Merged
agramfort merged 1 commit intoscikit-learn:masterfrom
tguillemot:correct_dpgmm_deprecation
Aug 6, 2016
Merged

[MRG+2] Correct Deprecation of DPGMM and VBGMM.#7124
agramfort merged 1 commit intoscikit-learn:masterfrom
tguillemot:correct_dpgmm_deprecation

Conversation

@tguillemot
Copy link
Copy Markdown
Contributor

The deprecation of VBGMM is not done correctly indeed when we call VBGMM we have two deprecation messages: one for VBGMM one for DPGMM.
Consequently I have modify the code (as we have done before for the GMM class) to be sure that the right deprecation warning is launched.

@agramfort @ogrisel @amueller Can you have a look please ?

@agramfort
Copy link
Copy Markdown
Member

LGTM

@tguillemot tguillemot changed the title Correct Deprecation of DPGMM and VBGMM. [MRG+1] Correct Deprecation of DPGMM and VBGMM. Aug 1, 2016
with ignore_warnings(category=DeprecationWarning):
g.fit(X)
trainll = g.score(X)
if isinstance(g, mixture.DPGMM):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why that change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

DPGMM and VBGMM are both deprecated and as VBGMM inherit from DPGMM you have two warnings messages when you call VBGMM.
I've corrected that to make sure the deprecation warning is meaningfull and moreover to be sure to pass the test once I will modify the deprecation message to refer to the new BayesianGaussianMixture class.

@agramfort
Copy link
Copy Markdown
Member

@amueller merge if you're happy.

+1 on my side

@tguillemot
Copy link
Copy Markdown
Contributor Author

@amueller @ogrisel I really need this to be merged before you review #6651.

@tguillemot
Copy link
Copy Markdown
Contributor Author

@ngoix If you have some time to review this, it's related to the Bayesian Gaussian Mixture.

@ngoix
Copy link
Copy Markdown
Contributor

ngoix commented Aug 5, 2016

This looks good to me!

@tguillemot tguillemot changed the title [MRG+1] Correct Deprecation of DPGMM and VBGMM. [MRG+2] Correct Deprecation of DPGMM and VBGMM. Aug 5, 2016
@tguillemot
Copy link
Copy Markdown
Contributor Author

@agramfort @ngoix Thanks.

@agramfort agramfort merged commit 05dfe6c into scikit-learn:master Aug 6, 2016
TomDLT pushed a commit to TomDLT/scikit-learn that referenced this pull request Oct 3, 2016
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.

4 participants