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