search

Bioconductor / BiocCheck

http://bioconductor.org/packages/BiocCheck
8 stars 26 forks source link

implement single colon typo check, refers to #60 #92

Closed LiNk-NY closed 4 years ago

LiNk-NY commented 4 years ago

Tries to make some savings by only calling BiocManager::available when there is a detection of : in the parsed code. See #60 for more details

LiNk-NY commented 4 years ago

It looks ready to go ! Thanks

Kayla-Morrell commented 4 years ago

Hey @LiNk-NY there seem to be some warning being generated from checkSingleColon. There is a package I'm reviewing (https://github.com/Bioconductor/Contributions/issues/1516) where the build report writing a warning, but not an explicit WARNING from BiocCheck https://github.com/Bioconductor/Contributions/issues/1516#issuecomment-648700382.

I did a little debugging and was able to trace down that there are instances where the developer uses single colon but not for importing a function. For example,

for(i in 2:length(trunkPath)){

and,

nodeRange <- c(Ntip(tree)+1):(length(tree$edge.length)+1)

There probably can be some code clean up on the part of the developer (I haven't done a full review yet) but I figured I'd bring it to your attention. The function isn't producing a wrong output, because these examples shouldn't produce a WARNING from the function. But within the function these examples are throwing a warning at the following line. https://github.com/Bioconductor/BiocCheck/blob/e0176f6591c771774f6cb05d4ff8bc1b145ad10e/R/checks.R#L1050 Let me know if you need more detail to help clear this up.

LiNk-NY commented 4 years ago

See #98