Skip to content
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

Analysis update: Add embryonal workflow to README #463

Merged
merged 7 commits into from
Nov 30, 2023

Conversation

zzgeng
Copy link

@zzgeng zzgeng commented Nov 10, 2023

Purpose/implementation Section

What scientific question is your analysis addressing?

Add embryonal workflow to README

What was your approach?

Add embryonal workflow to README

What GitHub issue does your pull request address?

Directions for reviewers. Tell potential reviewers what kind of feedback you are soliciting.

Which areas should receive a particularly close look?

embryonal workflow

Is there anything that you want to discuss further?

Is the analysis in a mature enough form that the resulting figure(s) and/or table(s) are ready for review?

Yes

Results

What types of results are included (e.g., table, figure)?

What is your summary of the results?

Reproducibility Checklist

  • The dependencies required to run the code in this pull request have been added to the project Dockerfile.
  • This analysis has been added to continuous integration.

Documentation Checklist

  • This analysis module has a README and it is up to date.
  • This analysis is recorded in the table in analyses/README.md and the entry is up to date.
  • The analytical code is documented and contains comments.

@zzgeng zzgeng requested a review from a team as a code owner November 10, 2023 20:01
@zzgeng zzgeng removed the request for review from AntoniaChroni November 16, 2023 16:26
Copy link

@JosephDybas JosephDybas left a comment

Choose a reason for hiding this comment

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

This figure is very good. It's a lot of information to fit into what is, I think, a very intuitive chart. I do have some specific suggestions as well as a general concern. First, in general, I don't think that the README.md file directly explains the subtyping that is reflected in the diagram. For example, the first criteria listed in the README is TTYH1 fusion but in the image that is depicted until the third layer of nodes. I would suggest moving all the pathology information first, since that comes first in the figure. I also think there are some criteria missing in the README such as the possibilities involving LIN28A, FOXR2 or chr19 amplification. Also, the definitions of the subtypes "EMTR, C19MC0altered", "ETMR, NOS" etc are missing from the text description.
As for specific comments on the layout of the figure

  1. I suggest you combine the "Overexpression of LIN28A" nodes and make "chr19 amplification" separate. I think you can just have two separate arrows going to subtype "ETMR, C19MC-altered".
  2. I also suggest you combine the FOXR2 nodes. How it is depicted now appears to be two conditions in serial rather than the "OR" requirement. I don't dispute the subtyping strategy but think it is counterintuitive to have the two nodes in different layers.

Nice job!

@zzgeng
Copy link
Author

zzgeng commented Nov 20, 2023

criteria listed in the README is TTYH1 fusion but in the image that is depicted until the third layer of nodes. I would suggest moving all the pathology information first, since that comes first in the figure. I also think there are some criteria missing in the README such as the possibilities involving LIN28A, FOXR2 or chr19 amplification. Also, the definitions of the subtypes "EMTR, C19MC0altered", "ETMR, NOS" etc are missing from the text description.
As for specific comments on the layout of the figure

Thank you for your suggestions! I have made changes to the figure to make it more readable. I also added a chunk of text in README to briefly describe how the molecular subtyping is performed. Please take a look when you have time and let me know if any update is needed.

Copy link
Member

@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.

The part below "pathology dx is nbl" is not correct, and actually I do not think that NBL needs to be in here at that location. You are capturing it with CNS NB-FOXR2.

I do have the same comment about the subtype boxes- I don't like the general "subtype follows methyl subtype" since you are not stating the exact subtypes here - can you put these each into the category they follow and remove the general boxes? Also add the scores?

@zzgeng zzgeng requested a review from jharenza November 27, 2023 17:22
- **CNS HGNET-MN1**: samples contain _MN1_ gene fusion.
- **CNS NB-FOXR2**: samples contain _FOXR2_ gene fusion or the overexpression of _FOXR2_.
- **CNS Embryonal, NOS**: Pathology diagnosis is "Neuroblastoma" and do NOT have high confidence methylation subtypes.
- For all the other samples with Pathology diagnosis of "Neuroblastoma" and have high confidence methylation subtypes, molecular subtypes of these samples follow their methylation molecular subtypes.

Choose a reason for hiding this comment

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

This isn't reflected in the image. The image contains a node for methylation classification calling "ETMR, C19MC-altered" but no other subtype options. If there are other subtypes based on methylation data I would include a reference in the README. I couldn't find this specfically in the WHO guidebook. If the subtype option is only ETMR, C19MC-altered I would somehow make the arrow extend to that node rather than have two subtype nodes classifying the same subtype.

Copy link
Author

Choose a reason for hiding this comment

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

Here the situation is little complicated.... I think the workflow needs to read from up to bottom, left to right. I am not sure how to make it clear to viewers. But in this case, ETMR, C19MC-altered is first identified by chr19 amp and fusion, followed by subtyping other samples. Then for all the samples that do not have molecular subtypes, we look at the methyl classification. We only have samples with ETMR, C19MC-altered high confidence methylation. That's why I only put ETMR, C19MC-altered in the end. In my opinion, it does not conflict with what is written in README. Please let me know if that makes sense.

Copy link

@JosephDybas JosephDybas left a comment

Choose a reason for hiding this comment

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

I think this image is very much improved. I just have one other comment about the methylation-based subtypes. Thanks!

Copy link
Member

@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.

For the box that says "samples do not have high-conf methyl subtypes" - Can you just remove that, since we aren't looking at all methyl (eg CNS BCOR, etc).

@jharenza jharenza merged commit 523d1a2 into dev Nov 30, 2023
21 of 22 checks passed
@jharenza jharenza deleted the add_embryonal_workflow branch November 30, 2023 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants