Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DT should not be an imported package #2112

Closed
wch opened this issue Sep 16, 2019 · 6 comments
Closed

DT should not be an imported package #2112

wch opened this issue Sep 16, 2019 · 6 comments

Comments

@wch
Copy link
Member

wch commented Sep 16, 2019

Because DT is an imported package, when you load devtools, it also loads htmltools, which makes it hard to do development on htmltools, especially on on Windows.

There are also some packages that are listed as Imports in crosstalk's DESCRIPTION but don't load immediately because they're not imported in the NAMESPACE. Even though they're not loaded immediately, these packages do need to be installed when DT (and thus crosstalk) is installed. Two of these packages have a pretty heavy installation footprint: shiny and ggplot2.

So there really two annoying issues: one is that loading devtools causes DT and htmltools to load. That one can be solved by removing importFrom(DT,datatable) from NAMESPACE. The other one is the heavy installation footprint. To solve that, DT would have to be removed from the Imports in the DESCRIPTION file. I see in #2085 that it was moved there specifically to cause it to be automatically installed. Maybe it could be changed to a Suggested package again and prompt for installation when it's needed?

@jimhester
Copy link
Member

jimhester commented Sep 16, 2019

devtools is a meta package intended to be installed on rarely on developers' machines, where dependency load is not really a concern, but making setup easy for those new to developing R packages is.

The code previously did prompt for installation when it was needed, but I don't think these concerns warrant changing it back to the previous behavior.

67a44f1 removes DT from an explicit import, which should resolve the htmltools concern.

@wch
Copy link
Member Author

wch commented Sep 16, 2019

I understand that devtools is a meta package, but this seems excessive for getting a development setup from a clean system:

> install.packages('devtools')
Installing package into ‘/Users/winston/R/3.7’
(as ‘lib’ is unspecified)
also installing the dependencies ‘zeallot’, ‘colorspace’, ‘utf8’, ‘vctrs’, ‘plyr’,
‘labeling’, ‘munsell’, ‘RColorBrewer’, ‘fansi’, ‘pillar’, ‘pkgconfig’, ‘httpuv’,
‘xtable’, ‘sourcetools’, ‘gtable’, ‘reshape2’, ‘scales’, ‘tibble’, ‘viridisLite’,
‘BH’, ‘sys’, ‘ini’, ‘backports’, ‘ps’, ‘lazyeval’, ‘shiny’, ‘ggplot2’, ‘later’, ‘askpass’,
‘clipr’, ‘clisymbols’, ‘curl’, ‘fs’, ‘gh’, ‘purrr’, ‘rlang’, ‘rprojroot’, ‘whisker’, ‘yaml’,
‘processx’, ‘R6’, ‘assertthat’, ‘rex’, ‘htmltools’, ‘htmlwidgets’, ‘magrittr’,
‘crosstalk’, ‘promises’, ‘mime’, ‘openssl’, ‘prettyunits’, ‘xopen’, ‘brew’,
‘commonmark’, ‘Rcpp’, ‘stringi’, ‘stringr’, ‘xml2’, ‘evaluate’, ‘praise’, ‘usethis’,
‘callr’, ‘cli’, ‘covr’, ‘crayon’, ‘desc’, ‘digest’, ‘DT’, ‘ellipsis’, ‘glue’, ‘git2r’, ‘httr’,
‘jsonlite’, ‘memoise’, ‘pkgbuild’, ‘pkgload’, ‘rcmdcheck’, ‘roxygen2’,
‘rstudioapi’, ‘rversions’, ‘sessioninfo’, ‘testthat’, ‘withr’

Warning in install.packages :
  unable to access index for repository https://cloud.r-project.org/bin/macosx/el-capitan/contrib/3.7:
  cannot open URL 'https://cloud.r-project.org/bin/macosx/el-capitan/contrib/3.7/PACKAGES'
Packages which are only available in source form, and may need compilation of
  C/C++/Fortran: ‘colorspace’ ‘utf8’ ‘vctrs’ ‘plyr’ ‘fansi’ ‘httpuv’
  ‘sourcetools’ ‘reshape2’ ‘scales’ ‘tibble’ ‘sys’ ‘backports’ ‘ps’ ‘lazyeval’
  ‘later’ ‘askpass’ ‘curl’ ‘fs’ ‘purrr’ ‘rlang’ ‘yaml’ ‘processx’ ‘htmltools’
  ‘promises’ ‘mime’ ‘openssl’ ‘commonmark’ ‘Rcpp’ ‘stringi’ ‘xml2’ ‘covr’
  ‘digest’ ‘ellipsis’ ‘glue’ ‘git2r’ ‘jsonlite’ ‘pkgload’ ‘roxygen2’ ‘testthat’
Do you want to attempt to install these from sources? (Yes/no/cancel) 

@jimhester
Copy link
Member

Maybe you know someone on the shiny team who can work on reducing these dependencies in DT and crosstalk ;)

FWIW covr does not need shiny to generate or serve the reports, it simply uses DT to create the tables for the static HTML, which I guess I could generate without the DT package, but it doesn't seem like a great use of my time.

@wch
Copy link
Member Author

wch commented Sep 20, 2019

I have to admit that you have a good point about the crosstalk dependencies. I filed an issue at rstudio/crosstalk#72.

@stefanoborini
Copy link

stefanoborini commented Oct 15, 2019

Dependency of devtools against a web framework is a major design flaw and should be addressed as a bug. I don't want shiny in my environment that has nothing to do or test with shiny.

Opened #2133 or I am forking devtools to create something that does not contain such dependency.

@lock
Copy link

lock bot commented Apr 15, 2020

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants