added todo comments for clarification #15
Conversation
|
@cansavvy this should be ready to review! Please let me know if this style of review works for you or if you'd like to see something different |
There was a problem hiding this comment.
@danieljgroso Thanks for these comments! This works quite nicely! Do you think you could take a whack at describing the things mentioned in the review?
This is where if you can dump any knowledge you have about these kinds of datasets we can help you craft it so it is helpful in this vignette when people are trying to learn this analysis!
example data caller (comment)
There was a problem hiding this comment.
Couple comments but thanks for these additions @danieljgroso ! I think we're close!
vignettes/getting-started.Rmd
Outdated
| ```{r} | ||
| library(magrittr) | ||
| library(gimap) | ||
| ``` | ||
|
|
||
| ## Data requirements | ||
|
|
||
| TODO: What kind of data would someone need to run this? How much flexibility is there in what the experimental set up might look like? |
There was a problem hiding this comment.
@sobrien29 this might be a good question to address with Alice and the broader team; e.g. how many timepoints are required for minimum functionality, will we require the pgPEN library, etc
There was a problem hiding this comment.
I generally agree with the idea let's develop this for the Berger lab first and then we can try to expand this package's flexibility as we move along. But totally, this is probably a bigger and ongoing discussion amongst the team!
There was a problem hiding this comment.
At minimum we need a T0 (or plasmid) and a later timepoint (actual timing is dependent mainly on cell growth kinetics). Intermediate timepoints can be included for a time-based analysis if wanted and I believe the test data includes one intermediate timepoint.
There was a problem hiding this comment.
@sobrien29 How would you describe this to a user in this context? What is a T0? That means just the first time point at 0 days? Do you have a paper we can link to that give people the basics of CRISPR screens?
Co-authored-by: Daniel J. Groso <95438884+danieljgroso@users.noreply.github.com>
There was a problem hiding this comment.
This is great! Just a few more things to wrap this up @danieljgroso and @sobrien29
- Delete all the TODOs (assuming they've been properly addressed) -- @cansavvy and @danieljgroso addressed this in 12ef572
- Can you add resources and links that would be helpful for context here?
- Can you address added todo comments for clarification #15 (comment) by adding information in the vignette?
Thanks! We're close!
Hey @cansavvy, would you be able to clarify point 3?
|
|
@danieljgroso Yes! So this conversation: #15 (comment) If you can add information into the vignette that clarifies the terminology of T0 and etc and what time line is typical for this kind of data collection? |
Description
Changes are non-functional in nature and include TODO comments. These comments are primarily serving to clarify some of the comments and descriptions of the code functionality and metadata.
Type of change
How Has This Been Tested?
N/A
Checklist: