Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Added methods for sex determination tool #40

Merged
merged 15 commits into from
Sep 16, 2019

Conversation

shrivatsk
Copy link
Contributor

Purpose

Added methods for sex determination tool

Issue

What GitHub issue does your pull request address?

Pull review checklist

The PR will be considered ready for review when all four items have been checked.

Copy link
Collaborator

@cgreene cgreene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some specific suggestions that I think should be made and also some questions about the specifics of what the reported_sex field is (re: gender or sex).

content/03.methods.md Outdated Show resolved Hide resolved
content/03.methods.md Outdated Show resolved Hide resolved
content/03.methods.md Outdated Show resolved Hide resolved
content/03.methods.md Outdated Show resolved Hide resolved
content/03.methods.md Outdated Show resolved Hide resolved
content/03.methods.md Outdated Show resolved Hide resolved
content/03.methods.md Outdated Show resolved Hide resolved
shrivatsk and others added 10 commits September 3, 2019 09:25
Co-Authored-By: Casey Greene <cgreene@users.noreply.github.com>
Co-Authored-By: Casey Greene <cgreene@users.noreply.github.com>
Co-Authored-By: Casey Greene <cgreene@users.noreply.github.com>
Co-Authored-By: Casey Greene <cgreene@users.noreply.github.com>
Co-Authored-By: Casey Greene <cgreene@users.noreply.github.com>
Co-Authored-By: Casey Greene <cgreene@users.noreply.github.com>
Co-Authored-By: Casey Greene <cgreene@users.noreply.github.com>
Copy link
Collaborator

@cgreene cgreene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need a couple more pairs of eyes on this. I would approve with these changes, but I want more folks to look at this before we merge.

content/03.methods.md Outdated Show resolved Hide resolved
content/03.methods.md Outdated Show resolved Hide resolved
content/03.methods.md Show resolved Hide resolved
shrivatsk and others added 2 commits September 11, 2019 15:33
Co-Authored-By: Casey Greene <cgreene@users.noreply.github.com>
Co-Authored-By: Casey Greene <cgreene@users.noreply.github.com>
Copy link
Member

@jaclyn-taroni jaclyn-taroni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some services use the term 'genetic sex' to refer to sex chromosome status inferred from DNA data (ref). We could use that term here.

content/03.methods.md Outdated Show resolved Hide resolved
content/03.methods.md Outdated Show resolved Hide resolved
shrivatsk and others added 2 commits September 16, 2019 12:06
Co-Authored-By: Jaclyn Taroni <jaclyn.n.taroni@gmail.com>
Co-Authored-By: Jaclyn Taroni <jaclyn.n.taroni@gmail.com>
We used the fraction of total normalized X and Y chromosome reads that were attributed to the Y chromosome as a summary statistic.
We reviewed this statistic in the context of reported gender and determined that a threshold of less than 0.2 clearly delineated female samples.
Fractions greater than 0.4 were predicted to be males.
Samples with values in the range [0.2, 0.4] were marked as unknown.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Samples with values in the range [0.2, 0.4] were marked as unknown.
Samples with values in the range [0.2, 0.4] were deemed as contaminated and removed from the dataset.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this reminds me - we never added manuscript text about NGScheckmate, which is our major means of QC...I will add a ticket!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is another mechanism that also suggests that these samples are contaminated, that might not be the best place to discuss filtering due to contamination. An alternative explanation could be that some of these samples are from individuals with more than two sex chromosomes. I don't know that we want to go into that level of detail in this paragraph, but I don't think this is the point where we want to say that the samples are contaminated.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm good point. This sample was really removed due to NGScheckmate.

Copy link
Collaborator

@jharenza jharenza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shrivatsk can you accept the change about contaminated samples that were removed? Thanks!

@jharenza
Copy link
Collaborator

@shrivatsk can you accept the change about contaminated samples that were removed? Thanks!

Disregard this!

@jharenza
Copy link
Collaborator

@cgreene are you ok merging now or do you want more eyes still?

Copy link
Collaborator

@cgreene cgreene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@jharenza jharenza merged commit ac9b7cd into AlexsLemonade:master Sep 16, 2019
cgreene pushed a commit that referenced this pull request Sep 16, 2019
This build is based on
ac9b7cd.

This commit was created by the following Travis CI build and job:
https://travis-ci.com/AlexsLemonade/OpenPBTA-manuscript/builds/127903074
https://travis-ci.com/AlexsLemonade/OpenPBTA-manuscript/jobs/236217043

[ci skip]

The full commit message that triggered this build is copied below:

Merge pull request #40 from shrivatsk/patch-2

Added methods for sex determination tool
cgreene pushed a commit that referenced this pull request Sep 16, 2019
This build is based on
ac9b7cd.

This commit was created by the following Travis CI build and job:
https://travis-ci.com/AlexsLemonade/OpenPBTA-manuscript/builds/127903074
https://travis-ci.com/AlexsLemonade/OpenPBTA-manuscript/jobs/236217043

[ci skip]

The full commit message that triggered this build is copied below:

Merge pull request #40 from shrivatsk/patch-2

Added methods for sex determination tool
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants