ccagc / QDNAseq

QDNAseq package for Bioconductor
47 stars 27 forks source link

error with poolRuns #112

Closed jpoell closed 2 months ago

jpoell commented 1 year ago

The poolRuns function now produces an error due to matrixStats::colMeans2 requiring a matrix, whereas the inputted phenoData is an annotated data frame. It was able to fix it with by assigning a modified version of poolRuns in my local environment. While I don't expect many users use poolRuns, I think it would be good to fix this nonetheless. Cheers, Jos

HenrikBengtsson commented 1 year ago

Thanks for reporting. This looks like a bug to me. I was first confused that this hasn't been caught yet, because we do have example("poolRuns", package = "QDNAseq") that calls poolRuns(). But, it turns out that that example never reaches the problematic code:

https://github.com/ccagc/QDNAseq/blob/41d4da2458593272964ce2121dc8df1b38f78bc6/R/poolRuns.R#L96-L97

Instead, it returns early on at:

https://github.com/ccagc/QDNAseq/blob/41d4da2458593272964ce2121dc8df1b38f78bc6/R/poolRuns.R#L53-L57

I'll try to fix.

HenrikBengtsson commented 1 year ago

Would you mind sharing your local patch?

jpoell commented 1 year ago

Sure, no problem!

From: Henrik Bengtsson @.> Sent: zaterdag 14 januari 2023 0:29 To: ccagc/QDNAseq @.> Cc: Poell, J.B. (Jos) @.>; Author @.> Subject: Re: [ccagc/QDNAseq] error with poolRuns (Issue #112)

Would you mind sharing your local patch?

- Reply to this email directly, view it on GitHubhttps://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fccagc%2FQDNAseq%2Fissues%2F112%23issuecomment-1382582266&data=05%7C01%7Cj.poell%40amsterdamumc.nl%7Cc16d27907a1842ad791208daf5bde92d%7C68dfab1a11bb4cc6beb528d756984fb6%7C0%7C0%7C638092493251803275%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=0rZBpCsVy0HaGT%2BChVq6LUnPTSP%2BNtNy%2Fr0tq%2F5SszM%3D&reserved=0, or unsubscribehttps://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAGETLDPNRDHWABXMQT3UXXLWSHQKVANCNFSM6AAAAAAT2OY52Y&data=05%7C01%7Cj.poell%40amsterdamumc.nl%7Cc16d27907a1842ad791208daf5bde92d%7C68dfab1a11bb4cc6beb528d756984fb6%7C0%7C0%7C638092493251803275%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2BwJn92IXkoxhcYEsRbz2lh%2BKA9uzwJvV4oyoxHemAOY%3D&reserved=0. You are receiving this because you authored the thread.Message ID: @.**@.>>


AmsterdamUMC disclaimer : www.amsterdamumc.org/disclaimers

briarj commented 1 year ago

Hi Hendrik, I applied the fix from Jos already, and it seems to work. However, I run into a new minor issue. I run binReadCounts with pairedEnds=TRUE (since we have paired-end data). Then, when I use poolRuns the class of @phenoData@data$paired.ends becomes "character" instead of "logical", and therefore throws the following error when I try to combine: Error in combine(pDataX, pDataY) : data.frames contain conflicting data:non-conforming colname(s): paired.ends

Greets, Arjen

HenrikBengtsson commented 1 year ago

I applied the fix from Jos already, and it seems to work

I'm waiting for that one

HenrikBengtsson commented 9 months ago

Sure, no problem!

@jpoell, we never received your patch here. Not sure how @briarj got a hand of it.

HenrikBengtsson commented 2 months ago

Fix in QDNAseq 1.41.2, that should soon be available in the Bioc devel branch.