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

Encourage Imports not Depends #3076

Open
mattdowle opened this issue Sep 26, 2018 · 20 comments
Open

Encourage Imports not Depends #3076

mattdowle opened this issue Sep 26, 2018 · 20 comments
Milestone

Comments

@mattdowle
Copy link
Member

mattdowle commented Sep 26, 2018

CRAN + BioC: Depends Imports


I've only recently realized how bad Depends: is, thanks to Jan's importing vignette. I just made its discouragement stronger : 51590bc

We could disallow Depends. This would also be beneficial to cedta()'s awkward implementation on its last line where it needs to do the tryCatch just for packages which Depend; that line could be removed.

But before we disallow Depends, we'd need to ask 69 CRAN packages to change from Depends to Imports. (Most revdeps already Import.) The longer we leave it, the greater the potential for new packages using Depends to be added to CRAN and the harder it will be to change.

Check when changed to Imports and published:

CRAN

  • Ac3net
  • acdcR
  • AF
  • bdots
  • bea.R
  • behavr
  • birankr
  • CBRT filed issue
  • cellKey
  • cffdrs
  • chicane
  • circhelp
  • classifierplots filed PR
  • colocPropTest
  • CoSMoS
  • coveffectsplot
  • damr
  • dbi.table filed issue
  • dbWebForms
  • dfmeta
  • DiDforBigData
  • didimputation
  • DiSCos
  • easycsv fixed in dev
  • EBPRS
  • edl
  • EGM
  • eyeTrackR
  • FeatureImpCluster
  • FOCI
  • fplot
  • fplyr
  • gbp filed PR
  • gdxdt
  • GenomicTools.fileHandler
  • GenoScan
  • heims
  • HospitalNetwork
  • HPLB
  • IBRtools
  • immunarch
  • intervalaverage
  • KMD
  • KPC
  • libbib
  • limexhub
  • LKT
  • lookupTable
  • lori filed PR
  • LSPFP
  • metaforest
  • microseq
  • miLineage
  • mpactr filed issue
  • musica
  • nlpred
  • nosoi
  • opendataformat filed issue
  • orgR
  • panelaggregation
  • pgTools
  • pkggraph
  • PreProcessRecordLinkage
  • qreport
  • rasterDT
  • rblt
  • rcprd
  • reclin2
  • reinsureR
  • rMIDAS
  • robCompositions
  • RSauceLabs
  • rusquant filed issue
  • RWildbook
  • SeaVal
  • shinyML
  • skm
  • slim
  • solaR2 filed issue
  • SOMnmR
  • somspace
  • sqlHelpers
  • tablecompare
  • textTools
  • TrumpetPlots
  • twangRDC
  • twl
  • WebAnalytics
  • WGScan
  • word.alignment
  • ZIprop

Bioconductor

More metadata on these packages: CRAN links, version control links. Sorted by recent publish date to prioritize those that are actively maintained.

Package Last CRAN Update Version GH URL
dbi.table 2024-12-10 1.0.1 GH
opendataformat 2024-12-04 2.1.1 GH
lori 2024-11-18 2.2.3 GH
CBRT 2024-11-13 0.1.1 GH
rcprd 2024-11-13 0.0.1
rusquant 2024-09-16 1.1.4 GH
solaR2 2024-09-16 0.10 GH
mpactr 2024-09-10 0.1.0 GH
DiSCos 2024-07-23 0.1.1 GH
circhelp 2024-07-04 1.1 GH
SOMnmR 2024-07-04 0.3.0 GH
LKT 2024-07-01 1.7.0
SeaVal 2024-06-14 1.2.0 GH
colocPropTest 2024-06-11 0.9.1
qreport 2024-05-26 1.0-1
EGM 2024-05-23 0.1.0
limexhub 2024-05-06 0.1.5
immunarch 2024-03-18 0.9.1 GH
cffdrs 2024-02-22 1.9.0
rblt 2024-02-19 0.2.4.7 GH
nosoi 2024-02-09 1.1.2 GH
reclin2 2024-02-09 0.5.0 GH
metaforest 2024-01-26 0.1.4
coveffectsplot 2024-01-18 1.0.5 GH
cellKey 2023-11-24 1.0.2 GH
tablecompare 2023-11-14 0.1.1 GH
sqlHelpers 2023-10-14 0.1.2
rMIDAS 2023-10-11 1.0.0 GH
WebAnalytics 2023-10-04 0.9.12 GH
PreProcessRecordLinkage 2023-09-13 1.0.1
robCompositions 2023-08-25 2.4.1
fplot 2023-08-24 1.1.0
fplyr 2023-08-23 1.3.0 GH
microseq 2023-08-21 2.1.6
IBRtools 2023-08-14 0.1.3
TrumpetPlots 2023-06-13 0.0.1.1
somspace 2023-04-28 1.2.4
DiDforBigData 2023-04-03 1.0 GH
pgTools 2023-03-24 1.0.2 GH
HospitalNetwork 2023-02-27 0.9.3 GH
bdots 2023-01-06 1.2.5 GH
rasterDT 2022-12-15 0.3.2 GH
libbib 2022-11-05 1.6.4
KMD 2022-10-06 0.1.0
KPC 2022-10-05 0.1.2
didimputation 2022-08-25 0.3.0
acdcR 2022-06-27 1.0.0 GH
chicane 2021-11-06 0.1.8
FeatureImpCluster 2021-10-20 0.1.5
edl 2021-09-20 1.1
ZIprop 2021-06-09 0.1.1 GL
CoSMoS 2021-05-29 2.1.0 GH
twangRDC 2021-05-17 1.0
FOCI 2021-03-18 0.1.3
shinyML 2021-02-24 1.0.1 GH
textTools 2021-02-05 0.1.0
dbWebForms 2021-01-28 0.1.0
damr 2020-11-16 0.3.7 GH
classifierplots 2020-10-13 1.4.0 GH
EBPRS 2020-08-26 2.1.0
intervalaverage 2020-07-23 0.8.0
HPLB 2020-06-09 1.0.0
LSPFP 2020-05-13 1.0.3
eyeTrackR 2020-03-29 1.0.1
birankr 2020-03-23 1.0.1
GenomicTools.fileHandler 2020-03-05 0.1.5.9
nlpred 2020-02-23 1.0.1
gdxdt 2019-11-30 0.1.0
WGScan 2019-05-27 0.1
AF 2019-05-20 0.1.5
word.alignment 2019-04-15 1.1
behavr 2019-01-03 0.3.2 GH
GenoScan 2018-12-21 0.1
pkggraph 2018-11-15 0.2.3 GH
twl 2018-08-24 1.0
easycsv 2018-05-21 1.0.8 GH
RWildbook 2018-04-06 0.9.3
dfmeta 2018-03-27 1.0.0
miLineage 2018-03-23 2.1
Ac3net 2018-02-26 1.2.2
bea.R 2018-02-23 1.0.6
reinsureR 2018-02-20 0.1.0
heims 2018-01-25 0.4.0
slim 2017-05-15 0.1.1
gbp 2017-01-28 0.1.0.4 GH
skm 2017-01-23 0.1.5.4 GH
panelaggregation 2017-01-07 0.1.1
RSauceLabs 2016-09-27 0.1.6 GH
musica 2016-09-03 0.1.3
lookupTable 2015-08-28 0.1
orgR 2014-12-20 0.9.0

These packages had previously used Depends: data.table but either no longer do, or were removed from CRAN:

List as code for easier maintenance of this list:

library(httr)
library(tools)
library(data.table)

if (requireNamespace("devtools", quietly=TRUE) && requireNamespace("BiocManager", quietly=TRUE)) {
  cran_depends = devtools::revdep('data.table', 'Depends')
  bioc_depends = devtools::revdep('data.table', 'Depends', bioconductor = TRUE)
} else {
  cran_depends = package_dependencies("data.table", reverse=TRUE, which="Depends")$data.table

  bioc_vr = '3.20'
  bioc_db = available.packages(repos = file.path('http://bioconductor.org/packages', bioc_vr, 'bioc'))
  bioc_depends = package_dependencies("data.table", bioc_db, reverse=TRUE, which="Depends")$data.table
}

desc_data <- function(package) {
  repo = getOption('repos')["CRAN"]
  desc_url <- file.path(repo, "web", "packages", package, "DESCRIPTION")
  desc_file <- httr::content(httr::GET(desc_url), encoding = "UTF-8")
  desc_conn <- rawConnection(desc_file, open = "r")
  on.exit(close(desc_conn))

  # based on very exhaustive GitHub CRAN mirror search of DESCRIPTION files
  url_fields = c("URL", "URLNote", "BugReports", "Github")
  desc = read.dcf(desc_conn, c("Package", "Date/Publication", "Version", url_fields))
  repo_regex = ".*git(?:hub|lab)[a-zA-Z0-9._]+/([A-Za-z0-9._]+/[A-Za-z0-9._]+).*"
  url = desc[,url_fields] |>
    Filter(f = Negate(is.na)) |>
    strsplit(",") |>
    lapply(function(str) {
      repo_like = grep(repo_regex, str, value=TRUE)
      repo_like = grep("github.io", repo_like, value=TRUE, invert=TRUE)
      gsub("/issues", "", trimws(repo_like))
    }) |>
    unlist() |>
    unique()

  out = data.table(
    package = desc[,"Package"],
    published_date = as.Date(as.POSIXct(desc[,"Date/Publication"])),
    version = desc[,"Version"])
  if (!length(url)) url = NA_character_
  if (length(url) > 1L) {
    # e.g. cellKey has a special BugReports repo
    url = grep(package, url, fixed=TRUE, value=TRUE)
    if (length(url) > 1L) stop("found more than one apparent URL for ", package)
  }
  out[, url := url]
  out[]
}

cran_depends = rbindlist(lapply(cran_depends, desc_data))

writeLines(paste("- [ ]", cran_depends$package))

cran_depends[, package := sprintf('[%1$s](https://cran.r-project.org/web/packages/%1$s/index.html)', package)]
cran_depends[, url := fcase(
  grepl("github", url), sprintf("[GH](%s)", url),
  grepl("gitlab", url), sprintf("[GL](%s)", url),
  !is.na(url), url
)]

setorder(cran_depends, -published_date)
setnames(cran_depends, c("Package", "Last CRAN Update", "Version", "URL"))
options(knitr.kable.NA = '')
knitr::kable(cran_depends)

writeLines(sprintf("- [ ] [%1$s](https://www.bioconductor.org/packages/%2$s/bioc/html/%1$s.html)", bioc_depends, bioc_vr))
@mattdowle mattdowle added this to the 1.12.0 milestone Sep 26, 2018
@jangorecki
Copy link
Member

jangorecki commented Sep 27, 2018

It shouldn't be disallowed because there are valid use cases for Depends, for example my package dtq which is a logging mechanism for [.data.table. I recall other package too: dt.nuggets(?). We should definitely recommend to imports instead of depends, but IMO it should not be disallowed as there are valid use cases for Depends.

@mattdowle
Copy link
Member Author

Fair enough. Why exactly is it that those packages need Depends and can't use Imports?

@jangorecki
Copy link
Member

They are extensions of data.table, their existence without data.table is pointless. They not just "use" data.table.

@mattdowle
Copy link
Member Author

mattdowle commented Sep 28, 2018

I still don't follow. Importing is as strong as depending in that imports are required. No package importing data.table can install without it. What would fail if they changed to Imports? There is also Enhances:.
Does dtq mask [.data.table so it need to be attached in front of data.table on the search path?

@jangorecki
Copy link
Member

jangorecki commented Sep 30, 2018

It is as strong, except the fact that imported namespace doesn't need to be attached. It doesn't mask, but it expects data.table to be attached. There are definitely very few use cases like this so general rule should be to import.
In case of listed rev deps that depends on data.table we could propose to change that to imports. Change might need to update examples and unit tests (thus all dependent scripts too) to require data.table explicitly together with such rev dep package, which is potentially significant breaking change, still easy to fix.
Regarding change of depends to imports of Rbitcoin rev dep, I am not maintaining this package anymore. It might eventually be incompatible with later versions of data.table or exchange market APIs, I have no idea. Latest CRAN release is 4+ years old, dev version is 2+ years old. If someone is willing to take maintenance of it then it will be best, otherwise it will probably gets removed from CRAN once it will start breaking to many newly added rules.

@mattdowle mattdowle modified the milestones: 1.12.0, 1.12.2 Jan 11, 2019
@mattdowle

This comment was marked as outdated.

@mattdowle mattdowle modified the milestones: 1.12.2, 1.12.4 Feb 27, 2019
@mattdowle mattdowle changed the title Disallow Depends:; require Imports: only. Encourage Imports not Depends Feb 27, 2019
@mbannert
Copy link

mbannert commented Feb 27, 2019

First of all thanks a ton for reaching out to the package maintainers.

I fully agree. I always use data.table for my backends and have imported on my more recent packages. In my case panelaggregation was a try to put a package on CRAN back when people wore pyjamas and lived life slow. Last time a data.table change caused issues I tried to get the package of CRAN but was discouraged to do so. I'd rather put my time into my newer packages than to fix a package which is more or less abandoned. On the other hand it would probably not be that much of deal to fix it though I haven't looked at the package in years.

Bottom line: from my point of view, disallowing would be a bit too much since it would get my old package of CRAN forcefully.

@tdhock
Copy link
Member

tdhock commented Feb 27, 2019

hi I just updated my penaltyLearning package, tdhock/penaltyLearning@ef8e7fb. now it imports data.table instead of depends. basically I just had to add a bunch of library(data.table) lines in tests and examples.

@bozenne
Copy link

bozenne commented Feb 28, 2019

Hi thanks for the email and for data.table
I have also changed to imports in the Github version of the BuyseTest package.
Will be updated on CRAN at some point.

Best
brice

@MichaelChirico

This comment was marked as outdated.

brown-jason added a commit to USEPA/CompTox-ToxCast-tcpl that referenced this issue Jun 5, 2019
And updated files to pass devtools::check()
@brown-jason
Copy link

tcpl package has moved data.table to imports

@mattdowle

This comment was marked as outdated.

@mattdowle mattdowle modified the milestones: 1.12.4, 1.13.0 Sep 19, 2019
@ekstroem
Copy link

Updated networkR package with Depends -> Imports on its way to CRAN

@jangorecki

This comment was marked as outdated.

@bozenne
Copy link

bozenne commented Nov 18, 2019

Hello,
I have (finally) updated the version of the BuyseTest package on CRAN (1.8)
where data.table has been moved to the imports section, as you suggested.

Regards
brice

@JWiley
Copy link

JWiley commented Nov 22, 2019

Hello,

This is fixed in JWileymisc now propagating through CRAN. I took the opportunity to change all depends into imports.

Cheers,

Josh

@mattdowle mattdowle modified the milestones: 1.12.7, 1.12.9 Dec 8, 2019
@alexWhitworth
Copy link

synthACS now uses Imports instead of Depends. Push sent to CRAN.

carlosparadis added a commit to sailuh/kaiaulu that referenced this issue May 27, 2020
Removing the packages from Depends on DESCRIPTION
required adding @importsFrom and @export across all
functions, hence this commit slightly modifies the
entire codebase.

Vignettes were edited to require() instead of attaching
using library() following best practies.

Rather than copy and paste data.table across all functions,
I followed XGBoost package approach and loaded only on a
single file. Since all codebase wont work without parsers,
that seem the most logical place. Other more situational
dependence on functions will use the typical :: approach
which is hard for data.table.

Arguably data.table could belong on Depends, but even the
package maintainer argues against it, see:

Rdatatable/data.table#3076
@mattdowle mattdowle modified the milestones: 1.13.1, 1.13.3 Oct 17, 2020
@jangorecki jangorecki modified the milestones: 1.14.3, 1.14.5 Jul 19, 2022
@mattdowle

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests