Feedback on simData

[JetteS]

1. Purpose of the package

The purpose of the package is stated in the title of the package and the README file on GitHub. However, the title and description in the DESCRIPTION file are the same and do only say "Simulate and analyze DNA" (please change analyze to analyse ;-) ). I would have liked a single paragraph describing more of what the package does under description. Furthermore, it would be nice, if the package had a package documentation (help file) with more information about what the package does and what functions are included.

After looking into the package, I feel that the title/name of the package and the descriptions are a little bit misleading. I expected a package that includes several functions for simulating different types of DNA data. Instead, the package does only include one single function that simulates a segregating sites matrix. The remaining functions focus on the site frequency spectrum and the mutation rate.

2. Completeness

• As already mentioned, I expected a different type of functions. If the package is supposed to help the user simulate and analyse DNA sequences, there should be more functions that actually simulate different types of data. Otherwise, I would change the name and description of the package to fit the actual package. Apart from this I felt that there were to few functions included in the package.

• There is no data included in the package, but I did not miss any type of data. All help files display examples with different types of input, and it was not hard to come up with my own examples.

• The DESCRIPTION file is almost complete. The only thing I found was that the point License does still say "License: What license is it under?". Please change that.

3. Code quality and sophistication

• The package did install immediately and without any problems. I used all functions with different examples, and it seems that the functions are robust.

• The package includes a single S3 method, namely print.population. This function does nothing else than the basic print function, as it simply prints the matrix x. Therefore, print.population() is not necessary and can be excluded from the package. In addition, the user has to change the class of the matrix manually by saying class(x) <- "population". It would have been nice if the package provided a constructor for the user. There is another function simDNAseq which seems to be a S3 method. But as before, the use of classes is not necessary here, as it simply reflects the type of simulation used.

4. Documentation

• All functions are documented and all help files provide examples. I noticed that all of the functions that are based on the site frequency spectrum have the same text in the paragraph Details in the help file: The site frequency spectrum is a vector of length n-1, where n is the sample size. The i'th entry is the number of segregating sites where a mutation occur in i sequences, and thus all entries must be natural numbers (0 included). You could consider changing the text a little bit in the different help files :-) I liked that you have a reference in all help files!

• The vignette is not included in the package, hence I had to knit it myself.\ Apart from this, it is very nice that you have two separate vignettes, one for the analysis and one for the simulation. You use the same examples in the vignette and the help files. I would have liked to see some different examples in the vignette ( maybe some more elaborate examples).

5. Interface

• All function (except for simDNAseq) are very simple and take only a single argument. Most names are appropriate and most arguments are named in a consistent way. I wondered why you let x denote the segregating sites matrix in print.population. It would be more natural to call it segSites (as in the function SNP). In addition, it was confusing that you have a SNP matrix and a segregating sites matrix. The difference is only explained in the help file for SNP.

• The functions check for user mistakes and give warnings when unreasonable arguments are used. In addition all error messages and warnings are easy to understand.

6. (optional) Additional comments

I already mentioned that I miss some more functions in the package. The function pairwDiff is already implemented as a part of the function mutRate and can be omitted. The function print.population is a replication of the basic print function and can be omitted, too. Hence, the package does only include five rather simple functions.

7. Conclusion

I really liked the help files and the vignettes. The package could become very useful, if you add some more sophisticated functions.

[JetteS]

The vignettes were included after all! I forgot to say build_vignettes=TRUE… Sorry :-)

[UnicRain]

Thank you for the feedback. We have updated the description file as suggested and we agree the name is misleading. We will implement better ways to use S3 objects.

In regard to the amount of functions, our focus was to have few well-tested, well-documented and user-friendly functions to practice different aspects of package-writing.

We appreciate your feedback.

[JetteS]

I understand that you wanted to focus on having few but well-tested and well-documented functions. You absolutely managed to do that! I think it would be nice if you mention that in the README file or in the package documentation, as it is clearly an advantage of your package.