GreenleafLab / ArchR

ArchR : Analysis of Regulatory Chromatin in R (www.ArchRProject.com)
MIT License
388 stars 140 forks source link

Potential pitfall when the subset indices is a Rle vector of TRUE and FALSE #1994

Closed andyyhchen closed 1 year ago

andyyhchen commented 1 year ago

https://github.com/GreenleafLab/ArchR/blob/c61b0645d1482f80dcc24e25fbd915128c1b2500/R/AllClasses.R#L723

The Rle vector will not go in any of the if condition and will be send to the unique(). This will make the Rle list become a two element list of TRUE and FALSE. Adding i <- as.vector(i) before if (is.logical(i)) will fix this.

Here is the code on how to reproduce this bug. proj[proj@cellColData$Sample == 'Sample1',] and this will end up subseting half of the proj cuz the broadcasting of c(TRUE, FALSE).

But user should be safe if they always access cellColData using $ sign since the dollar sign operator function does as.vector(x) before returning the values. https://github.com/GreenleafLab/ArchR/blob/c61b0645d1482f80dcc24e25fbd915128c1b2500/R/AllClasses.R#L679

Hence proj[proj$Sample == 'Sample1',] will output the correct subset.

rcorces commented 1 year ago

Hi @andyyhchen! Thanks for using ArchR! Please make sure that your post belongs in the Issues section. Only bugs and error reports belong in the Issues section. Usage questions and feature requests should be posted in the Discussions section, not in Issues.
It is worth noting that there are very few actual bugs in ArchR. If you are getting an error, it is probably something specific to your dataset, usage, or computational environment, all of which are extremely challenging to troubleshoot. As such, we require reproducible examples (preferably using the tutorial dataset) from users who want assistance. If you cannot reproduce your error, we will not be able to help. Before going through the work of making a reproducible example, search the previous Issues, Discussions, function definitions, or the ArchR manual and you will likely find the answers you are looking for. If your post does not contain a reproducible example, it is unlikely to receive a response.
In addition to a reproducible example, you must do the following things before we help you, unless your original post already contained this information: 1. If you've encountered an error, have you already searched previous Issues to make sure that this hasn't already been solved? 2. Did you post your log file? If not, add it now. 3.__ Remove any screenshots that contain text and instead copy and paste the text using markdown's codeblock syntax (three consecutive backticks). You can do this by editing your original post.

rcorces commented 1 year ago

thanks for posting. This has been updated in the dev branch. Worth noting that we no longer recommend subsetting projects like this and encourage users to use subsetArchRProject() which is more stable