privefl / bigsnpr

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

Using bigsnpr C++ headers in my package #460

Open kristjanmoore opened 8 months ago

kristjanmoore commented 8 months ago

I am quite new to C++, Rcpp, and package creation, so apologies if what I'm writing here is incoherent.

I have written some C++ code for an R package I'm developing which relies on bed-acc.h. It is an extension of bed_col_counts_cpp which I've called bed_2col_counts_cpp. It takes two vectors of column indices p and q, counts the occurrences of every possible genotype pair at each p_i and q_i ( 0 0, 0 1, 0 2, 0 missing, 1 0... ), and outputs a 16-row matrix with these counts. When I make a fork of the bigsnpr source code and add the function definition to bed-fun.cpp, it works well after installation.

I understand that when an R package exposes its C++ header files in an inst/include directory, it is quite easy to use those headers in new Rcpp code with lines like #include <mio/mmap.hpp> and #include <bigstatsr/utils.h>, as bigsnpr does. However, because bigsnpr itself doesn't have an inst/include directory, I think that the only robust solution I have for implementing bed_2col_counts_cpp in my package is to copy the bed-acc.h header into my package. This doesn't seem ideal, but I think this is what I will do for now (with attribution, of course).

My questions are 1) do you think that copying bed-acc.h an acceptable solution, 2) whether there's any chance that you could expose some of the bigsnpr headers in an inst/include directory which later versions of my package would #include, 3) whether there's some other way of implementing this bed_2col_counts_cpp function.

privefl commented 8 months ago

I can probably move this to inst/include. Would you like to try doing it and submit a PR? Otherwise I can probably do it tomorrow. In the meantime, feel free to copy this file for now.

kristjanmoore commented 8 months ago

Yes, it might be a good exercise for me to try to do that myself. I will try to get round to doing it at some point in the next couple of weeks. Thanks for letting me use the header for now, and thanks (as always!) for the prompt response.

privefl commented 8 months ago

Reopening as long as this is not done.

privefl commented 5 months ago

Any update on this?

privefl commented 2 months ago

Do you still want to do this?