-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
[ENH, FIX] Allow scaling_factor in trunk_mixture_classification to be specified #234
Conversation
Signed-off-by: Adam Li <adam2392@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #234 +/- ##
=======================================
Coverage 89.81% 89.81%
=======================================
Files 54 54
Lines 5026 5029 +3
=======================================
+ Hits 4514 4517 +3
Misses 512 512 ☔ View full report in Codecov by Sentry. |
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.
Why is 2/3 default? We don't have any simulations that use such scaling right now. Maybe in the future, but I would set the default to be identity.
cc: @sampan501 what was the reason again? Iforgot. |
I had done that before because when making the visualization, using 1 doesn't look bimodal. So, I stuck with the Marron and Wand default. For the sake of this though, I think we should just use 1 |
Signed-off-by: Adam Li <adam2392@gmail.com>
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.
Pending green CIs, lgtm
Signed-off-by: Adam Li <adam2392@gmail.com>
Changes proposed in this pull request:
Before submitting
section of the
CONTRIBUTING
docs.Writing docstrings section of the
CONTRIBUTING
docs.After submitting