Closed allyhawkins closed 4 months ago
I think this change is mostly fine, but I find it confusing that the umap_point_size and umap_facet_point_size are not defined in this file. I don't think it is the worst thing, as I know this is an included file, but it is a bit strange. I wonder if we shouldn't have a comment up at the top describing what this file is & requires.
I agree with this sentiment about it being confusing that the point sizes are defined in the other two files rather than here. I updated this to define the point sizes only in the file that is actually using that variable. So this moves it out of main_qc_report.rmd
and celltypes_supplemental_report.rmd
and instead defines those values in umap_qc.rmd
and celltypes_qc.rmd
.
I also updated the functions in celltypes_qc.rmd
to set the default point sizes to 1. And then I added a comment to the beginning of the celltypes_qc.rmd
that the report is intended to be included as a child report in the main and supplemental reports.
In #755 I made a fix to the UMAP point sizes variable that used the global value for a variable as the default for the function. I did that because I didn't see that variable defined anywhere in the report that uses that function, but it's defined in the main and supplemental report. With that change we need to actually define the point size when calling the functions to plot UMAPs. Otherwise, we get a failure when plotting a UMAP, since
point_size
isn't defined. So here, I'm just adding those lines. We could revert the change I made in #755, but I think this is more specific and easier to track what's being defined where, something that tripped me up when I was going back and making fixes in #755.