revdep TrialEmulation fails re-building vignettes due to new na.print arg

[tdhock]

Hi @joshhwuu and @MichaelChirico

It looks like #6087 has caused revdep TrialEmulation to fail re-building vignettes, see below

* checking re-building of vignette outputs ... ERROR
Error(s) in re-building vignettes:
  ...
--- re-building 'Getting-Started.Rmd' using rmarkdown

Quitting from lines  at lines 136-137 [unnamed-chunk-2] (Getting-Started.Rmd)
Error: processing vignette 'Getting-Started.Rmd' failed with diagnostics:
argument 3 matches multiple formal arguments
--- failed re-building 'Getting-Started.Rmd'

SUMMARY: processing the following file failed:
  'Getting-Started.Rmd'

Error: Vignette re-building failed.
Execution halted

it should be a simple fix in the downstream, https://github.com/Causal-LDA/TrialEmulation @joshhwuu could you please send them a PR to fix?

[joshhwuu]

I'll do after my exam tonight!

[MichaelChirico]

it should be a simple fix in the downstream,

In general, we should prefer fixing things inside data.table if possible, so I think first is to understand the root cause.

The problem is downstream is relying on partial argument matching here:

https://github.com/Causal-LDA/TrialEmulation/blob/989f064dfcb68f37ebdf280b2c37f8ee845aae0d/R/generics.R#L50

Previously n = 5 gets matched to nrows = 5, but with the addition of na.print to the signature, partial matching is no longer unique, hence the error.

In this case, I think downstream should fix by themselves, it's not recommended for package authors to rely on partial matching, this is one of many such good reasons.

So yes, I don't think data.table needs to do anything, and @joshhwuu that should make the PR to file with TrialEmulation pretty clear :)

If you want to go the extra mile, you can see if they are relying on partial matching anywhere else in their codebase.

[tdhock]

revdep checks say this is still an issue for TrialEmulation

[joshhwuu]

Follow up: Confirmed that version 0.0.3.8 on CRAN is before our last revdep fix, I installed v0.0.3.8 of TrialEmulation and was able to reproduce:

Source code from here, unzipped and opened: https://github.com/Causal-LDA/TrialEmulation/releases/tag/v0.0.3.8

install_github("Rdatatable/data.table")
check()

══ Documenting ════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
ℹ Installed roxygen2 version (7.3.2) doesn't match required (7.3.0)
✖ `check()` will not re-document this package
══ Building ═══════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Setting env vars:
• CFLAGS    : -Wall -pedantic -fdiagnostics-color=always
• CXXFLAGS  : -Wall -pedantic -fdiagnostics-color=always
• CXX11FLAGS: -Wall -pedantic -fdiagnostics-color=always
• CXX14FLAGS: -Wall -pedantic -fdiagnostics-color=always
• CXX17FLAGS: -Wall -pedantic -fdiagnostics-color=always
• CXX20FLAGS: -Wall -pedantic -fdiagnostics-color=always
── R CMD build ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
✔  checking for file ‘/home/joshhwuu/work/TrialEmulation-0.0.3.8/DESCRIPTION’ ...
─  preparing ‘TrialEmulation’:
✔  checking DESCRIPTION meta-information ...
─  cleaning src
─  installing the package to build vignettes
E  creating vignettes (32s)
   --- re-building ‘Getting-Started.Rmd’ using rmarkdown
   
   Quitting from lines 136-137 [unnamed-chunk-2] (Getting-Started.Rmd)
   Error: processing vignette 'Getting-Started.Rmd' failed with diagnostics:
   argument 3 matches multiple formal arguments
   --- failed re-building ‘Getting-Started.Rmd’
   
   SUMMARY: processing the following file failed:
     ‘Getting-Started.Rmd’
   
   Error: Vignette re-building failed.
   Execution halted

packageVersion("TrialEmulation")
# 0.0.3.8

[MichaelChirico]

revdep checks say this is still an issue for TrialEmulation

please see the comment and linked downstream PR above. this is fixed in their dev version but not CRAN; there's no back-compatible fix so they'll have to submit a new version after we do.