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

Tabula muris clean up #170

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Tabula muris clean up #170

wants to merge 19 commits into from

Conversation

aopisco
Copy link
Contributor

@aopisco aopisco commented May 2, 2018

in progress repo cleaning - stay tuned!

@aopisco aopisco self-assigned this May 2, 2018
@batson batson added the review label May 2, 2018
README.md Outdated
@@ -225,5 +225,5 @@ tabula_muris/
From Guoji Guo's lab at Zhejiang University School of Medicine in China

- [MCA Data Analysis](https://github.com/ggjlab/mca_data_analysis)
- [scMCA](https://github.com/ggjlab/scMCA) - An simple R package for large scale data (large DGE) from Mouse Cell Atlas ,to alleviate burdens of our main Server
- [scMCA](https://github.com/ggjlab/scMCA) - An simple R package for large scale data (large DGE) from Mouse Cell Atlas, to alleviate burdens of our main Server
Copy link
Contributor

Choose a reason for hiding this comment

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

This description is from them, doesn't make much sense here (what is "our main Server"?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I'll simplify that

writeData(wb, sheet = sheetname, dataframe,rowNames = FALSE)
}

saveWorkbook(wb,paste0(tissue,".xlsx"),overwrite = TRUE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this should be two spaces in as far as indentation goes.

```

```{r}
library(openxlsx)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not up to speed on the recent changes to this project overall so this might be a silly questions: what is the reason to list cell type markers by using an excel workflow?

If we do want to allow an excel workbook would we also want to support csv or another format? If not then ignore the rest, otherwise I would probably split this function to one that creates a dataframe with the needed values and returns that. Then create a separate function that would create an excel workbook given that dataframe. Then if someone wanted to write csvs or just explore the dataframe the function can be used for those purposes. Again this might not make any sense given the use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants