-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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] Correlations: fixes and enhancements #3591
Conversation
metas = [StringVariable("Feature 1"), StringVariable("Feature 2")] | ||
domain = Domain([ContinuousVariable("Correlation")], metas=metas) | ||
metas = list(map(StringVariable, ["Feature 1", "Feature 2"])) | ||
attrs = list(map(ContinuousVariable, ["Correlation", "p-Value"])) |
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 object to calling this a "p-value". I hope I haven't written https://blog.biolab.si/2019/01/04/how-to-abuse-p-values-in-correlations/ in vain. :)
As soon as we call this a p-value, people will call the top 5% correlations (or more, if the data is not random) statistically significant.
@BlazZupan, any ideas how to call this?
@@ -91,6 +91,7 @@ class CorrelationRank(VizRankDialogAttrPair): | |||
POSITIVE_COLOR = QColor(170, 242, 43) | |||
|
|||
threadStopped = Signal() | |||
pValRole = next(gui.OrangeUserRole) |
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'd prefer go with the naming style (not statistics) and call this PValRole
.
Well, I'd sure prefer calling this role something completely else, as noted in another comment.
Codecov Report
@@ Coverage Diff @@
## master #3591 +/- ##
=========================================
Coverage ? 84.03%
=========================================
Files ? 370
Lines ? 67080
Branches ? 0
=========================================
Hits ? 56368
Misses ? 10712
Partials ? 0 |
Codecov Report
@@ Coverage Diff @@
## master #3591 +/- ##
==========================================
+ Coverage 84.07% 84.11% +0.04%
==========================================
Files 370 370
Lines 67268 67364 +96
==========================================
+ Hits 56554 56664 +110
+ Misses 10714 10700 -14 |
3119631
to
c0cb43e
Compare
We decided to replace p-Value with False discovery rate. |
c0cb43e
to
07f38b7
Compare
07f38b7
to
daa333c
Compare
Issue
Fixes #3504
Heuristic will be updates in a following PR.
Description of changes
Includes