privefl / bigsnpr

R package for the analysis of massive SNP arrays.
https://privefl.github.io/bigsnpr/
183 stars 43 forks source link

`snp_autoSVD()` only works after a `library("bigsnpr")` call #501

Closed dramanica closed 1 month ago

dramanica commented 1 month ago

If using bigsnpr::snp_autoSVD() without first calling library("bigsnpr"), we get:

 object 'LD.wiki34' not found

This stems from line 119 in autoSVD.R, which is:

LRLDR <- LD.wiki34[0, 1:3]

Lazy loading does not happen in the function without previously loading the package. As the dataset is only used to initialise an empty data.frame, it would be better to simply use:

LRLDR <- data.frame(Chr=integer(), Start = integer(), Stop = integer())

so that the function can be used without the need to invoke library() first. Happy to create a pull request if it helps.

privefl commented 1 month ago

Thanks for reporting.

To reproduce:

bigsnp <- bigsnpr::snp_attachExtdata()
G <- bigsnp$genotypes
obj.svd <- bigsnpr::snp_autoSVD(G, bigsnp$map$chromosome)

Do you want to submit a PR?

dramanica commented 1 month ago

Sure, happy to make a PR. Two quick question: 1) Do you want a unit test? 2) Do you have a preference for bigsnpr::LD.wiki34 vs explicitely creating an empty data.frame?

privefl commented 1 month ago

Your way is probably more direct/explicit.

I am not sure we can have a unit test about this. Maybe can unload the package before testing the function? Anyway, we won't test this for the other functions. So, we can drop the unit test if you want.

dramanica commented 1 month ago

Ok, happy to go with the lazy option of no unit test, especially as, when creating the data.frame from scratch, the unit test does not make that much sense any more. PR submitted.