morinlab / GAMBLR

Set of standardized functions to operate with genomic data
https://morinlab.github.io/GAMBLR/
MIT License
3 stars 2 forks source link

Is genes an optional or required argument for prettyForestPlot + other prettyForestPlot issues #145

Closed rdmorin closed 1 year ago

rdmorin commented 1 year ago

The documentation says that genes is optional but when I run this function without it I get an error. Can the function be fixed to work without this option specified and/or the docs be updated to make this required?

prettyForestPlot(maf=bl_coding,metadata=bl_meth_meta,comparison_column="cluster",comparison_values=c("cluster1","cluster2"))
 Error in h(simpleError(msg, call)) : 
error in evaluating the argument 'table' in selecting a method for function '%in%': argument "genes" is missing, with no default
mattssca commented 1 year ago

I think it's best to update the documentation for this function so that it is clear that genes is a required parameter.

Specified genes in this parameter are required to filter the incoming maf and this is what the error you are seeing relates to.

In addition, there is a check at the beginning of this function that is meant to stop the function if no genes are provided. However, this currently does not work as intended. Will be fixed in a PR dedicated to resolving the issues raised for this function.

mattssca commented 1 year ago

There's another issue related to this plot:

prettyForestPlot(maf=bl_coding,metadata=bl_meth_meta,comparison_column="cluster",comparison_values=c("cluster1","cluster2"),genes=dlbcl_genes,max_q = 0.1)
FONT: 20 POINT: Inf 0
Error in `check_aesthetics()`:
! Aesthetics must be either length 1 or the same as the data (1): ymin, ymax, x and y
Run `rlang::last_error()` to see where the error occurred.
rdmorin commented 1 year ago

Wouldn't it be better to have it work as advertised though? Some folks may want to run the test on all genes rather than have to subset them upfront

mattssca commented 1 year ago

Of course, that sounds reasonable. I can add this functionality, no problem.

rdmorin commented 1 year ago

Resolved?

mattssca commented 1 year ago

Resolved!