leelabsg / SKAT

Sequence kernel association test (SKAT)
41 stars 16 forks source link

Suggest: no use of attach in examples #10

Closed richelbilderbeek closed 3 years ago

richelbilderbeek commented 3 years ago

Dear SKAT authors, thanks for writing this package! I do have a minor suggestion and I do volunteer to submit a Pull Request , which I describe below.

When viewing the SKAT::SKAT doc ...

library(SKAT)
?SKAT

we find this example code:

data(SKAT.example)
attach(SKAT.example)
# [...]
obj<-SKAT_Null_Model(y.c ~ 1, out_type="C")

I suggest to avoid the use of attach in this example, as lintr, the Tidyverse coding standard, states that attach is an undesirable function.

The easy solution would be to write the same code as such:

data(SKAT.example)
# [...]
obj <- SKAT_Null_Model(data = SKAT.example, formula = y.c ~ 1, out_type = "C")

In this way, the examples depend less on the state of env.

Could you agree that following the most used R coding standard is a good idea? Again, I do volunteer to do so :-)

richelbilderbeek commented 3 years ago

Contacted the SKAT maintainer by email:

Dear SKAT maintainer,

I contact you as I have posted two Issues at the SKAT GitHub repo and am unsure if you have read these.

If you have and just haven't had the time yet, I would understand :-)

Looking forward to a response one day and cheers, Richel Bilderbeek

leeshawn commented 3 years ago

Thanks for the suggestion. I modified the example

leeshawn commented 3 years ago

BTW, adding column name isn't done yet. It will be great if you can do this.

richelbilderbeek commented 3 years ago

Fixed with 4d12a26334bd628f438d92bd554aa5905a747517, thanks @leeshawn !