<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">
Thanks Lennart,
<div class=""><br class="">
</div>
<div class="">I suggest including Lars as one developer in our R-Forge project so that he can commit the package via SVN.</div>
<div class=""><br class="">
</div>
<div class="">Editing <a href="http://genabel.org" class="">genabel.org</a> sounds great. I wonder whether I should do the same for the MultiABEL package, but I think I can wait until the main papers get reviewed.</div>
<div class=""><br class="">
</div>
<div class="">Xia<br class="">
<br class="">
<div>
<blockquote type="cite" class="">
<div class="">On 06 Nov 2015, at 16:25, L.C. Karssen <<a href="mailto:l.c.karssen@polyomica.com" class="">l.c.karssen@polyomica.com</a>> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">Dear Lars, Xia,<br class="">
<br class="">
On 06-11-15 15:06, Lars Rönnegård wrote:<br class="">
<blockquote type="cite" class="">*Dear Lennart and Yurii,*<br class="">
<br class="">
*I attach the revised version of the RepeatABEL package following Xia’s<br class="">
review comments, which were very useful.*<br class="">
</blockquote>
<br class="">
Xia, thank you for doing the review!<br class="">
<br class="">
<br class="">
<blockquote type="cite" class=""><br class="">
* *<br class="">
<br class="">
*If you find my revision acceptable, I would be happy to have the<br class="">
package within the GenABEL suite of packages.*<br class="">
</blockquote>
<br class="">
<br class="">
This looks great! Thank you for the updates according to Xia's comments.<br class="">
We would be happy to accept RepeatABEL into the GenABEL suite.<br class="">
<br class="">
Some questions related to the GenABEL "membership":<br class="">
<br class="">
1) A technical question: would you like to save the RepeatABEL code in a<br class="">
public version control repository? The advantage would be that other<br class="">
people could more easily contribute to the development of the package,<br class="">
e.g. by fixing bugs.<br class="">
For the GenABEL Project we currently use the Subversion repository on<br class="">
R-forge [1] or Github [2].<br class="">
<br class="">
2) To increase the visibility of your package we can list it on<br class="">
<a href="http://www.genabel.org/packages" class="">http://www.genabel.org/packages</a>. Is that OK with you?<br class="">
<br class="">
3) In addition to the above we can give you access to the content<br class="">
management system running <a href="http://www.genabel.org" class="">www.genabel.org</a> so you can customise the<br class="">
RepeatABEL package page (see e.g. [3]), would you be interested in that?<br class="">
<br class="">
<br class="">
Finally, please take note of the responsibilities we expect from package<br class="">
maintainers [4].<br class="">
<br class="">
<br class="">
Best regards,<br class="">
<br class="">
<br class="">
Lennart.<br class="">
<br class="">
<br class="">
[1] <a href="https://r-forge.r-project.org/scm/?group_id=505" class="">https://r-forge.r-project.org/scm/?group_id=505</a><br class="">
[2] <a href="https://github.com/GenABEL-Project" class="">https://github.com/GenABEL-Project</a><br class="">
[3] <a href="http://www.genabel.org/packages/PredictABEL" class="">http://www.genabel.org/packages/PredictABEL</a><br class="">
[4] <a href="http://www.genabel.org/developers#maintainer_responsibilities" class="">
http://www.genabel.org/developers#maintainer_responsibilities</a><br class="">
<br class="">
<br class="">
<br class="">
<br class="">
<blockquote type="cite" class=""><br class="">
* *<br class="">
<br class="">
*Best regards,*<br class="">
<br class="">
*Lars Rönnegård*<br class="">
<br class="">
* *<br class="">
<br class="">
* *<br class="">
<br class="">
* *<br class="">
<br class="">
------------------------------------------------------------------------<br class="">
<br class="">
*From:* Xia Shen <<a href="mailto:xia.shen@ki.se" class="">xia.shen@ki.se</a> <<a href="mailto:xia.shen@ki.se" class="">mailto:xia.shen@ki.se</a>>><br class="">
*Sent:* Tuesday, October 27, 2015 06:19<br class="">
*To:* Lars Rönnegård<br class="">
*Cc:* L.C. Karssen<br class="">
*Subject:* Re: RepeatABEL<br class="">
<br class="">
<br class="">
<br class="">
Review of Package RepeatABEL v. 0.1<br class="">
<br class="">
Xia Shen<br class="">
<br class="">
Oct 26, 2015<br class="">
<br class="">
<br class="">
<br class="">
1. Introduction<br class="">
<br class="">
<br class="">
<br class="">
This is a novel contribution for the GenABEL project. Although the<br class="">
underlying model is more or less the same as polygenic_hglm + mmscore,<br class="">
the main motivation is that the GenABEL package does not deal with<br class="">
repeated measurements. The package is easy to use and provides an useful<br class="">
analytical framework for GWAS with repeated measurements, identical<br class="">
twins, clones, etc.<br class="">
<br class="">
<br class="">
<br class="">
2. Legal issues<br class="">
<br class="">
2.1. Is the copyright holder clearly mentioned?<br class="">
<br class="">
<br class="">
<br class="">
The author and maintainer are listed in DESCRIPTION and package help. It<br class="">
could also be displayed while loading the package.<br class="">
<br class="">
<br class="">
<br class="">
2.2. Is there a clear (standard) license?<br class="">
<br class="">
<br class="">
<br class="">
Yes. GPL.<br class="">
<br class="">
<br class="">
<br class="">
3. Technical quality<br class="">
<br class="">
3.1. Is the installation procedure clearly documented? Is the code easy<br class="">
to compile and run?<br class="">
<br class="">
<br class="">
<br class="">
Not documented in the tutorial, but easy to install as a standard R<br class="">
package. The compilation and basic running were successful.<br class="">
<br class="">
<br class="">
<br class="">
3.2. [For R packages] Does the package pass CRAN checks ()? At minimum,<br class="">
run "R CMD check …" and "R CMD check –as-cran …"<br class="">
<br class="">
<br class="">
<br class="">
The beginning of DESCRIPTION is redundant in CRAN check standard: 'The<br class="">
RepeatABEL package is used to'. <br class="">
<br class="">
<br class="">
<br class="">
R CMD check --as-cran produces:<br class="">
<br class="">
<br class="">
<br class="">
The Title field should be in title case, current version then in title case:<br class="">
<br class="">
‘GWAS for multiple observations on related individuals’<br class="">
<br class="">
‘GWAS for Multiple Observations on Related Individuals’<br class="">
<br class="">
<br class="">
<br class="">
The Date field is over a month old.<br class="">
<br class="">
<br class="">
<br class="">
Rd file 'preFitmodel.Rd':<br class="">
<br class="">
\usage lines wider than 90 characters:<br class="">
<br class="">
preFitModel(fixed=y~1, random=~1|id, id.name="id", genabel.data,<br class="">
phenotype.data, corStruc=NULL, GRM=NULL, Neighbor.Matrix=NULL)<br class="">
<br class="">
\examples lines wider than 100 characters:<br class="">
<br class="">
Mod1 <- preFitModel(fixed, random=~1|id, genabel.data = gen.data,<br class="">
phenotype.data = Phen.Data, corStruc=list( id=list("GRM","Ind") ))<br class="">
<br class="">
Mod2 <- preFitModel(fixed, random=~1|id + 1|nest, genabel.data =<br class="">
gen.data, phenotype.data = Phen.Data, corStruc=list( id=list("GRM","In<br class="">
... [TRUNCATED]<br class="">
<br class="">
Mod3 <- preFitModel(fixed, random=~1|id + 1|nest, genabel.data =<br class="">
gen.data, phenotype.data = Phen.Data, corStruc=list( id=list("GRM","In<br class="">
... [TRUNCATED]<br class="">
<br class="">
<br class="">
<br class="">
Rd file 'rGLS.rd':<br class="">
<br class="">
\usage lines wider than 90 characters:<br class="">
<br class="">
rGLS(formula.FixedEffects = y ~ 1, genabel.data, phenotype.data,<br class="">
id.name = "id", GRM = NULL, V = NULL, memory=1e8)<br class="">
<br class="">
<br class="">
<br class="">
Rd file 'simulate_PhenData.Rd':<br class="">
<br class="">
\usage lines wider than 90 characters:<br class="">
<br class="">
simulate_PhenData(formula.FixedEffects = y ~ 1, genabel.data,<br class="">
n.obs, SNP.eff = NULL, SNP.nr = NULL, beta = NULL, VC = c(1, 1, 1), GRM<br class="">
= ... [TRUNCATED]<br class="">
<br class="">
\examples lines wider than 100 characters:<br class="">
<br class="">
Phen.Sim <- simulate_PhenData(y ~ age,<br class="">
genabel.data=gen.data, n.obs=rep(4, nids(gen.data)), SNP.eff=1,<br class="">
SNP.nr=1000, VC=c(1,1,1) ... [TRUNCATED]<br class="">
<br class="">
<br class="">
<br class="">
These lines will be truncated in the PDF manual.<br class="">
<br class="">
<br class="">
<br class="">
Please fix.<br class="">
<br class="">
<br class="">
<br class="">
3.3. Is the package documented? What is the quality of the documentation?<br class="">
<br class="">
<br class="">
<br class="">
Yes. Good tutorial with standard Rd documentation.<br class="">
<br class="">
<br class="">
<br class="">
3.4. [For R packages] Does help(PackageName) provide an adequate summary<br class="">
of the package and a review of the major functions?<br class="">
<br class="">
<br class="">
<br class="">
Yes. But the displayed version number is different from what's in<br class="">
DESCRIPTION. Please check.<br class="">
<br class="">
<br class="">
<br class="">
3.5. [For R packages] Does the package use Roxygen2 for documentation?<br class="">
<br class="">
<br class="">
<br class="">
No. Roxygen2 is recommended, which keeps doc and code in the same R file<br class="">
and auto-generates Rd files.<br class="">
<br class="">
<br class="">
<br class="">
3.6. Are examples of usage provided?<br class="">
<br class="">
<br class="">
<br class="">
Yes.<br class="">
<br class="">
<br class="">
<br class="">
3.7. Does the package provide a tutorial/vignette? Can you comment on<br class="">
the tutorial?<br class="">
<br class="">
<br class="">
<br class="">
Yes. The tutorial is good and easy to follow. <br class="">
<br class="">
<br class="">
<br class="">
3.8. Is the source code of the tutorial/vignette provided?<br class="">
<br class="">
<br class="">
<br class="">
No. I recommend including the tutorial as a vignette.<br class="">
<br class="">
<br class="">
<br class="">
3.9. Does the package make use of unit/integration/etc. tests?<br class="">
<br class="">
<br class="">
<br class="">
No.<br class="">
<br class="">
<br class="">
<br class="">
3.10. [For R packages] Does the package make use of unit tests (e.g.<br class="">
RUnit or testthat)?<br class="">
<br class="">
<br class="">
<br class="">
No.<br class="">
<br class="">
<br class="">
<br class="">
3.11. Does the code comply with the GenABEL coding standards?<br class="">
<br class="">
<br class="">
<br class="">
Not entirely. Please refer<br class="">
to <a href="http://genabel.r-forge.r-project.org/codingstyle.html" class="">http://genabel.r-forge.r-project.org/codingstyle.html</a><br class="">
<br class="">
<br class="">
<br class="">
3.12. Is the code readable/understandable?<br class="">
<br class="">
<br class="">
<br class="">
More or less.<br class="">
<br class="">
<br class="">
<br class="">
3.13. Does the code contain explanatory comments?<br class="">
<br class="">
<br class="">
<br class="">
Yes.<br class="">
<br class="">
<br class="">
<br class="">
3.14. Were the design and methods implemented in package discussed<br class="">
during the development process (e.g. on the genabel-devel mailing list)?<br class="">
<br class="">
<br class="">
<br class="">
Yes. A little.<br class="">
<br class="">
<br class="">
<br class="">
4. Content<br class="">
<br class="">
4.1. Does the package address a problem in the domain of statistical<br class="">
genomics?<br class="">
<br class="">
<br class="">
<br class="">
Yes.<br class="">
<br class="">
<br class="">
<br class="">
4.2. Is it streamlining analyses not covered elsewhere in the GenABEL<br class="">
suite? If not, does it improve the analysis already covered?<br class="">
<br class="">
<br class="">
<br class="">
No and yes. <br class="">
<br class="">
<br class="">
<br class="">
4.3. Should it become a separate package or rather be incorporated into<br class="">
an existing package?<br class="">
<br class="">
<br class="">
<br class="">
A separate package is the current form and in this case would be easy to<br class="">
maintain in practice, although I also feel that it could actually be<br class="">
easy to integrate the main rGLS function into the GenABEL package.<br class="">
<br class="">
<br class="">
<br class="">
4.4. Does the package use any of the data types defined in other GenABEL<br class="">
packages?<br class="">
<br class="">
<br class="">
<br class="">
Yes. The gwaa.data class of GenABEL is used.<br class="">
<br class="">
<br class="">
<br class="">
4.5. Does the package use code/functions/data defined in other GenABEL<br class="">
packages?<br class="">
<br class="">
<br class="">
<br class="">
Yes.<br class="">
<br class="">
<br class="">
<br class="">
5. Recommendations<br class="">
<br class="">
5.1. What are the major issues that should be addressed?<br class="">
<br class="">
<br class="">
<br class="">
Nothing specifically more than above. After fixing the above issues, the<br class="">
package can be committed to GenABEL project R-Forge SVN.<br class="">
<br class="">
<br class="">
<br class="">
5.2. What other (optional) suggestions do you have for the author?<br class="">
<br class="">
<br class="">
<br class="">
Nothing specifically.<br class="">
<br class="">
<br class="">
<br class="">
On 26 Oct 2015, at 08:30, Lars Rönnegård <<a href="mailto:lrn@du.se" class="">lrn@du.se</a><br class="">
<<a href="mailto:lrn@du.se" class="">mailto:lrn@du.se</a>>> wrote:<br class="">
<br class="">
<br class="">
<br class="">
Xia,<br class="">
<br class="">
Have you had time to look at the package yet? I received the review<br class="">
on the paper today with minor revision and I am expected to submit<br class="">
the revision within 3 weeks so it would be great if I could have<br class="">
revised the package according to your comments by then.<br class="">
<br class="">
<br class="">
<br class="">
Best regards,<br class="">
<br class="">
Lars<br class="">
<br class="">
<br class="">
<br class="">
<br class="">
<br class="">
<br class="">
<br class="">
</blockquote>
<br class="">
-- <br class="">
*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*<br class="">
Lennart C. Karssen<br class="">
PolyOmica<br class="">
Groningen<br class="">
The Netherlands<br class="">
<br class="">
<a href="mailto:l.c.karssen@polyomica.com" class="">l.c.karssen@polyomica.com</a><br class="">
GPG key ID: 1A15AF2A<br class="">
-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-<br class="">
<br class="">
</div>
</blockquote>
</div>
<br class="">
</div>
</body>
</html>