Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

group.by set to NULL in combineExpression fails #294

Closed
michael-kotliar opened this issue Dec 21, 2023 · 1 comment
Closed

group.by set to NULL in combineExpression fails #294

michael-kotliar opened this issue Dec 21, 2023 · 1 comment

Comments

@michael-kotliar
Copy link

When setting group.by to NULL this line fails because comparing NULL and string doesn't produce any value. is.null(group.by) should go before group.by == "none"

https://github.com/ncborcherding/scRepertoire/blob/d188e6ffe71db03c7f79f86652c9b2378aeceffe/R/combineExpression.R#L79

> NULL == "none"
logical(0)
ncborcherding added a commit that referenced this issue Dec 21, 2023
@michael-kotliar pointed out that null evaluation should go before "none"
@ncborcherding
Copy link
Member

Hey Michael,

Thanks for pointing this out - you are correct. If you find anymore of these, feel free to make a pull request and get your username on the repo. I think it's important to keep track of that.

Also if you have larger suggestions or functions you might have, feel free to reach out via email. We are in the process of testing out v2 beta and will be writing a manuscript next year, would be happy to have any contribution.

Nick

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants