Enhance EvidenceQC outputs to better align with sample QC #728
Conversation
|
What's the reason for changing |
@mwalker174 Some samples have a value of NaN for certain metrics (e.g. |
| df_manta_low_outlier, df_melt_low_outlier, df_wham_low_outlier, df_total_low_outliers, | ||
| df_melt_insert_size] | ||
| for df in dfs: | ||
| df[ID_COL] = df[ID_COL].astype(object) | ||
| output_df = reduce(lambda left, right: pd.merge(left, right, on=ID_COL, how="outer"), dfs) | ||
| output_df = output_df[output_df[ID_COL] != EMPTY_OUTLIERS] | ||
| output_df = output_df.replace([None, np.nan], 0.0) |
There was a problem hiding this comment.
We do want to set samples who are not outliers in any categories to 0 in the outlier columns, rather than NaN, for interpretability. But I think it makes more sense to do that for just the outlier columns, not the entire dataframe. There may not be other metrics which routinely have NaNs at the moment, but if an NaN did show up in another metric (current or future), it would not necessarily make sense to set it to 0
There was a problem hiding this comment.
Updated accordingly - thanks for clarifying.
| filename: A tab-delimited file containing estimated copy numbers. | ||
| Returns: | ||
| A pandas DataFrame containing the following columns: | ||
| [id, chr1_CopyNumber, ..., chr22_CopyNumber, chrX_CopyNumber, chrY_CopyNumber, chrX_CopyNumber_rounded]. |
There was a problem hiding this comment.
I assume this documentation was a copy/paste error and should be updated to refer to the sex assignment data?
There was a problem hiding this comment.
Updated accordingly - thanks for catching this.
| df_manta_low_outlier, df_melt_low_outlier, df_wham_low_outlier, df_total_low_outliers, | ||
| df_melt_insert_size] | ||
| for df in dfs: | ||
| df[ID_COL] = df[ID_COL].astype(object) | ||
| output_df = reduce(lambda left, right: pd.merge(left, right, on=ID_COL, how="outer"), dfs) | ||
| output_df = output_df[output_df[ID_COL] != EMPTY_OUTLIERS] | ||
| output_df = output_df.replace([None, np.nan], 0.0) | ||
| output_df.rename(columns={ID_COL: NEW_ID_COL}, inplace=True) |
There was a problem hiding this comment.
Is there a reason for making this change after the fact, rather than updating the value of ID_COL?
Side note that it may be worth asking around if people have strong feelings about using # to indicate the header line in this file. I don't think it's necessary for this file, and I think you have the right idea about updating it for concordance with other files and ease of use with pandas, but it is a common convention in the field, which is why the original header had #ID.
There was a problem hiding this comment.
Yes, this was done mainly so that upstream files did not have to be changed to the new ID naming standard.
Now that you mention that this is a common convention though, I'm wondering if we can/should just leave it as is?
There was a problem hiding this comment.
Updated all upstream tables to use sample_id - have hence removed this renaming from the script.
Description
This PR is intended to introduce several enhancements to
qc_matrixoutput as part of theEvidenceQCworkflow that enable it to align better with the sample QC process, which is orchestrated by the QC Notebook. These enhancements are currently composed of the following:NaNto be0.sample_id.Testing
qc_matrixoutput from the 1KGP cohort.Pre-Merge Changes Required
Remove automated Dockstore image sync for development branch.