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

Initiate GOI module README #246

Merged
merged 5 commits into from
Dec 6, 2022
Merged

Conversation

cbethell
Copy link
Contributor

@cbethell cbethell commented Nov 1, 2022

What is the purpose of these changes?

This PR initiates the genes of interest (GOI) module documentation.

What changes did you make?

A README.md file is added to the optional-goi-analysis module here, so far with just an analysis overview section along with expected input and expected output.

Any comments, concerns, or questions important for reviewers

  • Is there any additional information you feel should be added here (keeping in mind that documentation referencing parameters and how to run workflow will be in a subsequent PR)?

Checklist

Place an x in all boxes that you have completed.

  • I have run the most recent version of the code
  • If I am adding in reports or generating any plots, I have reviewed the necessary reports
  • I have added all necessary documentation (if applicable)

@allyhawkins allyhawkins self-requested a review November 11, 2022 20:42
Copy link
Member

@allyhawkins allyhawkins left a comment

Choose a reason for hiding this comment

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

I took a first look and added some general comments. The biggest comment is about the overview of the actual workflow. We should describe the actual steps that are taken in the workflow rather then just the two scripts that are run.

optional-goi-analysis/README.md Outdated Show resolved Hide resolved

There are two main steps of this genes of interest analysis workflow:

1. **Genes of Interest Calculations**: Provided gene identifiers are first optionally mapped to a specified keytype.
Copy link
Member

Choose a reason for hiding this comment

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

I think it would make more sense to break these steps up into something that makes more sense to external users and not just the two steps that we do which is calculations and plots. What actually is happening in the script?

Maybe something like these headers?

  1. Mapping gene identifiers
  2. Heirarchical clustering of dataset with genes of interest - this is what the heatmap represents
  3. Visualization of genes of interest expression (Sina, PCA, and UMAP)

optional-goi-analysis/README.md Outdated Show resolved Hide resolved
optional-goi-analysis/README.md Outdated Show resolved Hide resolved
@cbethell
Copy link
Contributor Author

cbethell commented Dec 1, 2022

All good points @allyhawkins! I've re-structured the analysis overview headers, added a code chunk with the tree structure of the output, re-arranged some wording per the recent updates we made -- let me know what you think of things now!

@cbethell cbethell requested a review from allyhawkins December 1, 2022 21:23
Copy link
Member

@allyhawkins allyhawkins left a comment

Choose a reason for hiding this comment

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

Thanks for making these changes! I think there will still be some things we don't have fully fleshed out yet, but this mostly looks good. I think the biggest question I have is how we want to deal with explaining mapping gene identifiers.

optional-goi-analysis/README.md Outdated Show resolved Hide resolved
optional-goi-analysis/README.md Outdated Show resolved Hide resolved
optional-goi-analysis/README.md Outdated Show resolved Hide resolved
optional-goi-analysis/README.md Show resolved Hide resolved
Comment on lines 50 to 54
goi_stats
├── library01_mapped_genes.tsv
├── library01_normalized_zscores.mtx
├── library01_heatmap_annotation.rds
└── library01_goi-report-template.html
Copy link
Member

Choose a reason for hiding this comment

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

I think this might change when we get to the snakefile, as it would make sense to move the rendered html file out of the goi_stats folder, but this is fine for now.

optional-goi-analysis/README.md Outdated Show resolved Hide resolved
@cbethell
Copy link
Contributor Author

cbethell commented Dec 6, 2022

Thanks for the review @allyhawkins, I implemented those wording changes and will file an issue regarding a separate section to explain mapping gene identifiers. Otherwise let me know what you think now 👍

@cbethell cbethell requested a review from allyhawkins December 6, 2022 15:22
Copy link
Member

@allyhawkins allyhawkins 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! One more minor change but I don't need to see it again.

optional-goi-analysis/README.md Outdated Show resolved Hide resolved
@cbethell cbethell merged commit 074c2b6 into development Dec 6, 2022
@cbethell cbethell deleted the cbethell/initiate-goi-docs branch December 6, 2022 15:43
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.

2 participants