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

Add cellassign organs to report #609

Merged
merged 9 commits into from
Dec 7, 2023

Conversation

sjspielman
Copy link
Member

@sjspielman sjspielman commented Dec 7, 2023

Stacked on #608
Closes #589

Here, I've updated the cell type report to include organs. It turns out our current parsing of the reference name would not work well for all of our new references (e.g. adrenal-pancreas-reproductive or bone-and-soft-tissue), so I decided to just scrap that code altogether, figuring the tissues/organs are more important than our internal reference name. Yes/no?

Here's a screen shot example of how this part of the cell type report now looks!


Screenshot 2023-12-07 at 2 00 08 PM

@jashapiro
Copy link
Member

It turns out our current parsing of the reference name would not work well for all of our new references (e.g. adrenal-pancreas-reproductive or bone-and-soft-tissue)

I think we should update the parsing and and present both.

@sjspielman
Copy link
Member Author

Please also note I am prepared to die on the hill of using the Oxford comma.

@sjspielman
Copy link
Member Author

I think we should update the parsing and present both.

Actually, would we even need to do any sort of parsing here at all? Or just say, "here's the name of the reference we used and it contains these tissue types."

@jashapiro
Copy link
Member

I think we should update the parsing and present both.

Actually, would we even need to do any sort of parsing here at all? Or just say, "here's the name of the reference we used and it contains these tissue types."

If there is nothing additional, no, we don't need to parse it.


methods_text <- glue::glue(
"{methods_text}
* Annotations from [`CellAssign`](https://github.com/Irrationone/cellassign), a marker-gene-based approach ([Zhang _et al._ 2019](https://doi.org/10.1038/s41592-019-0529-1)).
Marker genes associated with `{tissue_type}` tissue were obtained from [PanglaoDB](https://panglaodb.se/).\n
"
Marker genes associated with the following tissue types were obtained from [PanglaoDB](https://panglaodb.se/): {organs_string}.\n"
Copy link
Member

@jashapiro jashapiro Dec 7, 2023

Choose a reason for hiding this comment

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

PangaloDB call the column organ, so I don't think we want to call it "tissue types". But "organ" seems wrong too. Maybe "Marker genes for cell types categorized to the following organs were obtained from PanglaoDB"

Copy link
Member Author

Choose a reason for hiding this comment

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

I have deleted and re-written like 4 sentence versions over here, so I'll take any wording suggestions I can get!

@sjspielman
Copy link
Member Author

Updated phrasing in 4828bce ... eh? feedback suuuper welcome.

Base automatically changed from sjspielman/589-cellassign-organ-description to development December 7, 2023 20: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.

I tried my hand at the wording suggestion 🤷‍♀️

templates/qc_report/celltypes_supplemental_report.rmd Outdated Show resolved Hide resolved
templates/qc_report/celltypes_supplemental_report.rmd Outdated Show resolved Hide resolved
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.

LGTM!

@sjspielman sjspielman merged commit 504a280 into development Dec 7, 2023
3 checks passed
@sjspielman sjspielman deleted the sjspielman/589-organs-to-report branch December 7, 2023 20:46
@allyhawkins
Copy link
Member

One other thing, I'm going to have to do another release for cell typing to account for the memory/ cpu increases before we can complete processing objects. Maybe you can file a PR with the changes from this and #608 to main so we can get this into that release? Up to you.

@allyhawkins allyhawkins mentioned this pull request Jan 2, 2024
12 tasks
allyhawkins pushed a commit that referenced this pull request Jan 3, 2024
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