-
Notifications
You must be signed in to change notification settings - Fork 83
Add pathology subtyping of glialneuronal tumors #1017
Add pathology subtyping of glialneuronal tumors #1017
Conversation
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.
LGTM, I ran the code and checked the output files and it satisfies the expected updates requested from from #996
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.
Hi @jaclyn-taroni ! The logic here looks perfect to me, but I have some requests and one request and one question.
I request to rename clinical-subtyping-glialneuronal-tumors.nb*
to pathology-harmonized-diagnosis-glialneuronal-tumors.nb*
since this data is derived from pathology reports and is going to become harmonized diagnoses.
My question here is similar to that on my review of #1016 - do we want to make these separate scripts or add to the 03-incorporate-pathology-feedback.nb.Rmd
script? (I know 03
is quite long, so I would be ok with adding separate scripts, and for samples subtyped, they should be added to compiled_molecular_subtypes_with_clinical_pathology_feedback.tsv
.
The case for these is a bit different - we will actually remove these from the subtyping file since they do not get subtyped, so I think we should add that step here.
@jharenza I don't follow - should I be adding something to the notebook I'm adding here? I will rename the notebook. As I said in #1016 (comment), we should prioritize making sure the logic is correct and then compile the results and reorganize the module as necessary. |
|
||
```{r} | ||
data_dir <- file.path("..", "..", "data") | ||
histologies_file <- file.path(data_dir, "pbta-histologies.tsv") |
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.
histologies_file <- file.path(data_dir, "pbta-histologies.tsv") | |
histologies_file <- file.path(data_dir, "pbta-histologies-base.tsv") |
I believe the scripts here should use pbta-histologies-base.tsv
since the pbta-histologies.tsv
is only created later in molecular-subtyping-integrate
module.
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 think that's probably true here but not in #1016 (https://github.com/AlexsLemonade/OpenPBTA-analysis/pull/1016/files#r625098165) since that's downstream of CRANIO subtyping... does that seem correct to you?
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.
Addressed in da8ae7b
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 logic now looks good to me and the N (14) is correct.
@jaclyn-taroni we should also add |
Done! When I ran this with v19 data, |
yes, this looks expected! |
Purpose/implementation Section
What scientific question is your analysis addressing?
Subtyping glialneuronal tumors per the WHO 2016 CNS guidelines & instructions on #996
What was your approach?
Here I'm adding a notebook to the
molecular-subtyping-pathology
module to do the subtyping. I'm using the exclusion criteria for LGAT subtyping (e.g., the JSON file from that module being altered in #1014) as inclusion criteria in this notebook.What GitHub issue does your pull request address?
Closes #996
Directions for reviewers. Tell potential reviewers what kind of feedback you are soliciting.
Which areas should receive a particularly close look?
The output –
analyses/molecular-subtyping-pathology/results/glialneuronal_tumor_subtypes.tsv
– should be carefully reviewed. As should the logic inanalyses/molecular-subtyping-pathology/clinical-subtyping-glialneuronal-tumors.Rmd
.Reproducibility Checklist
Documentation Checklist
README
and it is up to date.analyses/README.md
and the entry is up to date.