Closed kweav closed 3 months ago
First off, thanks for your great PR description. Really appreciate the rundown.
I think overall this looks great. ❤️
Thanks!
For me to give the feedback on the requested review questions (also great love this ❤️), can you let me know what example code I should run to test this? i.e. what code have you been running when you developed this? If I start with what you've been using to develop it will help guide me through what to review.
The example code that I ran was rendering the vignette
getting_started.Rmd
Requested Review:
- Is the code for the new plots robust enough?
Looks good on a first pass, main question is 3 - 5 column
That's a question I have as well, sorry if I forgot to notate it.
- How to fix the heatmap rendering in the wrong location I'll dig into this a bit on next round.
Thanks!
- Is the new
R/filters.R
file an ok addition, or would you rather I put those functions elsewhere?Can we add these to the
02-filter.R
file?Definitely can put them there!
@kweav I think all we need to do to merge this is:
R/filters.R
1) I can push the tidyverse way to this branch 2) Will be in a later PR. I added an example one in PR #35
R/filters.R
- I can push the tidyverse way to this branch
- Will be in a later PR. I added an example one in PR Start with adding parameters that allow the user to select which column(s) are used in various filtering or filtering-related visualization steps #35
Used commit edb662a to push change 1
@cansavvy pkgdown is happy now!
@cansavvy pkgdown is happy now!
YAY!!!! 🎉
This PR adds two graphs to the QC report as requested in the May 20th meeting.
Changes made in this PR to accomplish this goal:
R/qc-plots.R
file. One for the histogram (qc_variance_hist()
) and one for the bar plot (qc_constructs_countzero_bar()
). Both use the gimap_dataset and assume that replicates are stored in columns 3-5.inst/rmd/gimapQCTemplate.Rmd
file (and added some appropriate headers)R/filters.R
file where possible filter functions will be stored. Currently the only filter in there is the zero count filter (qc_filter_zerocounts()
)Open issues: You may notice that I removed the three dots for the heatmap plot when calling the function and within the function. When I tested my code locally and rendered the vignette to drive rendering the qc report, the heatmap was rendered within the vignette rather than the output qc report. So I was trying to troubleshoot that, but it's still an open issue that I wasn't able to resolve.
Next steps for upcoming stacked PRs:
R/filters.R
file.Requested Review:
R/filters.R
file an ok addition, or would you rather I put those functions elsewhere?