[Bioc-devel] Question about sumbition process and biocviews

[Martin Morgan]

On 11/06/2016 04:54 PM, Ioannis Vardaxis wrote:
> Thanks for the answer,
>
> I have some other questions about BiocCheck.
>
> First I get the following warning:
> * This is a software package, checking vignette directories...
>     * ERROR: 'vignettes' directory!
>
>
> I have my vignettes directory in the inst/doc/vignettes. If I change the
> directory and place the vignettes in the top directory I don¹t get an
> error from biocCheck, but I get a fatal error from Rcheck.

The vignettes directory should be in the top-level directory 
YourPacakge/vignettes/ and not in YourPackage/inst/doc. If you get a 
fatal error, then there is something about your vignette (or indirectly 
in your package code) that is very incorrect. You should be able to 
'tangle' your vignette to get the pure R code, and then to run it, e.g., 
I can

     $ cd Rsamtools/vignettes
     $ R CMD Stangle Rsamtools-Overview.Rnw
     Output file:  Rsamtools-Overview.R
     $ R -f Rsamtools-Overview.R

>
> Secondly:
> Checking unit tests...
>     * NOTE: Consider adding unit tests. We strongly encourage them. See
>       http://bioconductor.org/developers/how-to/unitTesting-guidelines/.
>
>
> I read about unit tests, however the package is done and it works fine,
> and at the same time I don¹t see how unit tests would benefit me based on
> the functions in my package. Are them necessary?

They are strongly encouraged but not required. Many would argue that 
they are necessary to ensure that your package behaves as expected, and 
continues to do so as underlying dependencies change. Here's a silly 
function

     f = function(n) {
         x = integer(n)
         for (i in 1:n)
             x[i] = i
         x
     }

and a unit test that reveals an error.

     test_that("f works as expected for common cases", {
         expect_equal(f(2), c(1L, 2L))
         expect_equal(f(1), 1L)
         expect_equal(f(0), integer(0))
         expect_error(f("two"))
     })

Which leads to

Error: Test failed: 'f works as expected for common cases'
* f(0) not equal to integer(0).
Lengths differ: 1 vs 0

The fix in this case might be

     f = function(n) {
         seq_len(n)
     }

though now its clear that my four lines of code were not necessary at all.

Interestingly, in writing that example I made a mistake in my original 
f(), writing x[i] = n rather than my intended x[i] = i. The first 
expectation failed, helping to show me what I was doing wrong. I also 
wrote the last expectation as f("2"), which a little surprisingly also 
failed (to create an error) -- R coerced 1:"2" to 1:2; maybe I'd want to 
think about whether this behavior would be appropriate in my actual 
code, and if not write some checks on the inputs. It's surprising how 
easy bugs creep into code, and how unit tests help to see those bugs. See

   http://bioconductor.org/developers/how-to/unitTesting-guidelines/

for some further motivation.

>
> Thirdly, I used the Rdpack for adding references to the roxynen files, but
> I get an error from biocCheck that the macro is unknown. Is there another
> way of using references in roxygen?

this isn't formulated in a clear enough way (e.g., with a short example 
and with the exact error message) to be able to provide an answer.

>
> Finally, a question about coding: My longest function is 326 lines long,
> and cannot be smaller, is that ok? I get a note in biocCheck. Moreover I
> get the following note:
> * Checking formatting of DESCRIPTION, NAMESPACE, man pages, R source,
>   and vignette source...
>     * NOTE: Consider shorter lines; 4 lines (0%) are > 80 characters
>       long.
>     * NOTE: Consider indenting lines with a multiple of 4 spaces; 462
>       lines (6%) are not.
>
>
> None of my code cross the 80 character line though and the spaces are
> using the double tab that you need.

These are style recommendations and generally 'at your discretion'. Of 
course your 326 line function can be smaller -- identify the logical 
steps of your computation ('check inputs', 'initialize', 'iterate to 
convergence', 'validate result', 'format result for user') and implement 
each as a (non-exported) function called by the the function exposed to 
the user. This way you can think carefully about the logic of each 
function, write extensive unit tests that validate the inputs and 
expected outputs of the function, and hopefully re-use code in several 
different places.

The second NOTE above suggests indentation with spaces rather than tabs 
(4 spaces per tab) simply because tab width (and hence visual 
indentation) is at the discretion of the editor in use.

Martin

>
>
> Best,
>


This email message may contain legally privileged and/or...{{dropped:2}}