-
Notifications
You must be signed in to change notification settings - Fork 22
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
Fix dsfdr and load repseqs from qiime2 #168
Conversation
amnona
commented
Nov 27, 2019
- Load repseqs file from qiime2 qza if supplied
- fix a bug in dsfdr when using a user supplied statist for dsfdr - we didn't take the abs() of the effect size so would only look for positive extreme effect size
calour/io.py
Outdated
logger.info('Rep seqs hashes and table hashes are not equal. Using table hashes.') | ||
newexp.feature_metadata.rename(columns={'_feature_id': '_hash'}, inplace=True) | ||
newexp.feature_metadata = newexp.feature_metadata.join(other=rep_seqs, on='_hash', how='left') | ||
newexp.feature_metadata.set_index('_feature_id', inplace=True, drop=False) | ||
|
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.
what does this change diff?
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.
This was added so the sequences (and not the hashes) are the _feature_id and index
(added comment in file)
@@ -292,11 +291,13 @@ def dsfdr(data, labels, transform_type='rankdata', method='meandiff', | |||
# call the user-defined function of statistical test | |||
t = method(data, labels) | |||
tstat = t.copy() | |||
t = np.abs(tstat) |
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 need to be absolute value? could you add comment to explain?
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.
We need abs() since dsFDR performs a two-sided test. added comment in file.
comments addressed. @RNAer - ready for merge? |
there are conflicts |
fixed the conflict (don't know why it created the conflict - same line in both versions....) |