Some vignette notes

[jasonserviss]

Congrats on the preprint! It caught my interest and I was having a look through the vignette and noticed some small typos/spelling etc. that I thought, since I was having a read through anyway, I would add here.

1. Section 2.2
   1.1 The first equation ends in [?]. Looks like this may be a Latex error. You may also want to define the symbols in the equation.
   1.2 "reguardless" in last sentence from section 2.2 should be "regardless".

2. Section 2.3.1
   2.1 This phrase "will throw errors and, subsequently, should be checked before running." from section 2.3.1 seems a bit off grammatically since "checked" cannot be both "subsequently" and "before". I think you want either a) "will throw errors and should be checked before running" or b) "will throw errors and, subsequently, should be checked". b) is better to my ears.
   2.2 There are several examples of using ... object where it is unclear what class the object is which might be more informative to the user. Looking at the class definition, it would appear that these are regular base R object classes (matrix, data.frame, etc.), so it might be better to just call them this e.g. "AnnotionObj a data frame with annotation for the data" or "AnnotionObj: data frame; includes data annotation".
   2.3 "Annotion" should be "Annotation"
   2.4 In the IDcol explanation it says "AnnotionData object" although this is previously referred to as "AnnotionObj" not "AnnotionData object". Would be better to adopt a consistent term. This may be rectified "automatically" by addressing a.
   2.5 "arguement" should be "argument"
   2.6 "whose rows annotation links", I believe "rows" should be singular.

3. Section 2.3.2.
   3.1 "For the the the base" should probably be "For the base" in addition this sentence would seem to be incomplete "For the the the base projectR function, Projection is the full lmFit model from the ."

4. Section 3.
   4.1 "maximized" should be "maximise"
   4.2 "and uncorrelated to previous components" should probably be "and uncorrelated to the previous components"
   4.3 "The projectR function has S3 method for class prcomp." should probably be "The projectR function has S3 method for the prcomp class."

5. Section 3.1 - 3.2. The code is running outside of the code box or page in several places. Bioconductor (I am assuming you are planning on submitting here since you are using BiocStyle) has a 80 character per line regulation so I am sure BiocCheck is already complaining about this but, at least in one instance, the entirety of the code cannot be seen making it difficult for a user to reproduce the example. When the code overruns the box it is also difficult to copy and paste it, which is what many of the people using the vignette are going to be doing. A tip from me is, if you use Rstudio as a code editor, you can go to Preferences -> Code -> Display and check "Show margin" and set "Margin column" to 80. The editor will now show a vertical line at 80 characters making it much easier to identify places where your code will overrun the code boxes (and where BiocCheck will complain).

It seems like you are still actively working on the package and I am sure you would have found many of these things on your own. I would really like to have a closer look at the package once the documentation has progressed a bit so, if you would like, you can update the issue when this has happened and I can have another look and help out as an additional "pair of eyes".

p.s. You may also want to add temporary installation instructions to the README (devtools::install_github('genesofeve/projectR')) until Bioconductor acceptance for people like me who are interested in trying out the package earlier.

[genesofeve]

Thank you so much! Cleaning up the package is the top of my to do list now that the preprint is out so this is super helpful. I will definitely take you up on your offer for another pair of eyes. There will be a lot of pushes in the days to come so hopefully you won’t be waiting too long.

On Tue, Aug 21, 2018 at 3:43 AM Jason Serviss ***@***.***> wrote: Congrats on the preprint! It caught my interest and I was having a look through the vignette and noticed some small typos/spelling etc. that I thought, since I was having a read through anyway, I would add here. 1. Section 2.2 1.1 The first equation ends in [?]. Looks like this may be a Latex error. You may also want to define the symbols in the equation. 1.2 "reguardless" in last sentence from section 2.2 should be "regardless". 2. Section 2.3.1 2.1 This phrase "will throw errors and, subsequently, should be checked before running." from section 2.3.1 seems a bit off grammatically since "checked" cannot be both "subsequently" and "before". I think you want either a) "will throw errors and should be checked before running" or b) "will throw errors and, subsequently, should be checked". b) is better to my ears. 2.2 There are several examples of using ... object where it is unclear what class the object is which might be more informative to the user. Looking at the class definition, it would appear that these are regular base R object classes (matrix, data.frame, etc.), so it might be better to just call them this e.g. "*AnnotionObj* a data frame with annotation for the data" or "*AnnotionObj:* data frame; includes data annotation". 2.3 "Annotion" should be "Annotation" 2.4 In the IDcol explanation it says "AnnotionData object" although this is previously referred to as "AnnotionObj" not "AnnotionData object". Would be better to adopt a consistent term. This may be rectified "automatically" by addressing a. 2.5 "arguement" should be "argument" 2.6 "whose rows annotation links", I believe "rows" should be singular. 3. Section 2.3.2. 3.1 "For the the the base" should probably be "For the base" in addition this sentence would seem to be incomplete "For the the the base projectR function, Projection is the full lmFit model from the ." 4. Section 3. 4.1 "maximized" should be "maximise" 4.2 "and uncorrelated to previous components" should probably be "and uncorrelated to *the* previous components" 4.3 "The projectR function has S3 method for class prcomp." should probably be "The projectR function has S3 method for *the prcomp class.*" 5. Section 3.1 - 3.2. The code is running outside of the code box or page in several places. Bioconductor (I am assuming you are planning on submitting here since you are using BiocStyle) has a 80 character per line regulation so I am sure BiocCheck is already complaining about this but, at least in one instance, the entirety of the code cannot be seen making it difficult for a user to reproduce the example. When the code overruns the box it is also difficult to copy and paste it, which is what many of the people using the vignette are going to be doing. A tip from me is, if you use Rstudio as a code editor, you can go to Preferences -> Code -> Display and check "Show margin" and set "Margin column" to 80. The editor will now show a vertical line at 80 characters making it much easier to identify places where your code will overrun the code boxes (and where BiocCheck will complain). It seems like you are still actively working on the package and I am sure you would have found many of these things on your own. I would really like to have a closer look at the package once the documentation has progressed a bit so, if you would like, you can update the issue when this has happened and I can have another look and help out as an additional "pair of eyes". p.s. You may also want to add temporary installation instructions to the README (devtools::install_github('genesofeve/projectR')) until Bioconductor acceptance for people like me who are interested in trying out the package earlier. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#12>, or mute the thread <https://github.com/notifications/unsubscribe-auth/APZDpDkbhNkBa6Aefq2zBspSqbk4BP__ks5uS7oygaJpZM4WFQwX> .

-- Sent from Gmail Mobile