-
Notifications
You must be signed in to change notification settings - Fork 118
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
Added examples to models in 'mixture' folder #722
base: main
Are you sure you want to change the base?
Conversation
Added examples to models in 'mixture' folder - GMMClassifier() - BayesianGMMClassifier() - GMMOutlierDetector() - BayesianGMMOutlierDetector()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mkalimeri , thanks for the PR and for your patience! Work has been a bit hectic recently.
This is very valuable and I would say that it is almost ready - I left comments on bayesian_gmm_classifier.py
, but similar feedback is true for all examples:
Examples
indentation- Avoid plotting functionalities in the docstrings
- Some minor code formatting
- Print results under each print statement
Examples | ||
-------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation should be:
Examples | |
-------- | |
Examples | |
-------- |
|
||
```python | ||
import numpy as np | ||
import matplotlib.pyplot as plt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can skip everything involving plotting, it won't be rendered anyway
|
||
# Classify a new point into one of two clusters | ||
p = np.array([[1.5, 0.5]]) | ||
p_prob= bgmm.predict_proba(p) # predict the probabilities p belongs to each cluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny format:
p_prob= bgmm.predict_proba(p) # predict the probabilities p belongs to each cluster | |
p_prob = bgmm.predict_proba(p) # predict the probabilities p belongs to each cluster |
Applied changes from p[ull request
Applied changes from review
Applied changes from review
Applied changes from review
Hello @FBruzzesi, thank you for the feedback! Working on issues is fun, I am happy to pick up more ;-) I committed the changes requested, but I see the tests are not passing. The errors appear to be related to other files, so I am not sure about how to proceed! Please let me know how things look. Thank you! |
Hey @mkalimeri thanks for fixing the issues! I will review it soon-ish! Don't worry about the test suite, we knew that scikit-learn 1.6 release would have broken our tests 🙃 we are working on a fix |
Fixes #596 (partially)
Description:
Added examples to models in 'mixture' folder
Type of change: Bug fix (non-breaking change which fixes an issue)
Before working on a large PR, please check with @FBruzzesi or @koaning to confirm that they agree with the direction of the PR. This discussion should take place in a Github issue before working on the PR, unless it's a minor change like spelling in the docs.
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
Fixes #(issue)
Type of change
Checklist:
If you feel your PR is ready for a review, ping @FBruzzesi or @koaning.