Skip to content

Bug in pwmType to find PWMs #23

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

Closed
jkzebrafish opened this issue Oct 21, 2017 · 4 comments
Closed

Bug in pwmType to find PWMs #23

jkzebrafish opened this issue Oct 21, 2017 · 4 comments
Assignees

Comments

@jkzebrafish
Copy link

Hi Alicia, great project, excited to get this to work!
Looks like the pwmType method has a small easily fixable bug.
I have:
version
pwmlistisok
But then:
error

This seems to be line 3 of pwmType(...) method, where the all.equal(...) call lacks the "current" argument, and hence always returns an error, which causes the isTRUE(...) call to by default return FALSE. Maybe it's something else but that seems like the first obvious thing to check.
pwmtype_line3bug

Thanks Alicia! Keep up the good work.
Best,
John Mich

@AliciaSchep
Copy link
Contributor

Thanks for posting! I'll take a look and update here when bug is fixed.

@AliciaSchep AliciaSchep self-assigned this Oct 22, 2017
@AliciaSchep
Copy link
Contributor

@jkzebrafish There is a current argument, rep(1, length(pwm), so that is not source of bug.

I think it is that there are column names on your pwm matrix, and that currently causes the colSums to not equal a plain vector of 1's without names. As a temporary fix, if you remove the column names then the function should work. I tested adding column names to the profile matrix and was able to reproduce the bug. You could remove them via colnames(TFBSTools::Matrix(cisbp_PWMatrixList[[1]])) <- NULL

I will fix the function to ignore column names.

@jkzebrafish
Copy link
Author

Oh ok that's great! Much easier than what I was thinking, will give it a try.

Thanks so much for your help Alicia!
John

@AliciaSchep
Copy link
Contributor

This should now be fixed in latest versions of both motifmatchr and chromVAR. Thanks for bringing up the issue!

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