search

Bioconductor / BiocCheck

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

Avoid downloading external data, refers to #75 #94

Closed LiNk-NY closed 4 years ago

LiNk-NY commented 4 years ago

The code searches for keywords that usually indicate download from external hosting platforms such as GitHub, GitLab, BitBucket.

LiNk-NY commented 4 years ago

I resolved some conflicts but the history now looks a bit convoluted. The changes have been preserved though.

lshep commented 4 years ago

When I try to run BiocCheck on a package I get the following 

* Checking individual file sizes...
Error in file.path(pkgDir, "DESCRIPTION") : object 'pkgDir' not found
Calls: <Anonymous> ... getPkgType -> .getViews -> file.exists -> file.path
Execution halted

This is also picked up if you run R CMD build and R CMD check on the package with your changes. It Errors in both Examples and Tests. 

* Checking individual file sizes...
Error in file.path(pkgDir, "DESCRIPTION") : object 'pkgDir' not found
Calls: BiocCheck ... getPkgType -> .getViews -> file.exists -> file.path
Execution halted
* checking for unstated dependencies in 'tests' ... OK
* checking tests ...
  Running ‘runTests.R’
 ERROR
Running the tests in 'tests/runTests.R' failed.
Last 13 lines of output:

  ERROR in test_getPkgType: Error in file.path(pkgDir, "DESCRIPTION") : object 'pkgDir' not found
  ERROR in test_vignettes0: Error in file.path(pkgDir, "DESCRIPTION") : object 'pkgDir' not found

  Test files with failing tests

     test_BiocCheck.R 
       test_checkForBadDepends 
       test_getPkgType 
       test_vignettes0 

  Error in BiocGenerics:::testPackage("BiocCheck") : 
    unit tests failed for package BiocCheck
  Execution halted

Please run R CMD build and R CMD check before submitting pull requests.

LiNk-NY commented 4 years ago

Hi Lori, @lshep Sorry about that. It should be fixed now. I've run build and check. Thanks

lshep commented 4 years ago

I have one that is downloading from dropbox. @mtmorgan / @LiNk-NY Thoughts on if we should add dropbox to the check too?

LiNk-NY commented 4 years ago

@lshep I think it would be good to add Dropbox as well. Perhaps, we should implement a more generic mechanism for internet downloads...? Not sure how feasible that is.

lshep commented 4 years ago

Perhaps. Maybe I'll hold off on merging this right now so we can explore this a little more?

mtmorgan commented 4 years ago

Yes, dropbox belongs on the list. Maybe a 'clear' list rather than a 'block' list is a better way to go -- acceptable download locations.

lshep commented 4 years ago

Is there any acceptable besides ExperimentHub? or an existing CRAN/Bioconductor data package?

LiNk-NY commented 4 years ago

Other API resources like CiVIC, cBioPortalData etc. could potentially be excluded if we don't have an extensive 'clear' list. There are some packages that use lower level httr calls to access data from a remote resource.
I think checking for the most common locations is good enough.

LiNk-NY commented 4 years ago

See #104 for the latest PR