Remove dependency on knitr and Suggest knitr instead

[andrie]

The checkpoint package only directly depends on knitr. In turn knitr depends on several other packages (see the dependency graph below). We have already removed several other dependencies from the package.

knitr is used to detect library() and require() calls in Rmarkdown in your project. It is a nice but absolutely essential feature, and we could conceivably remove this.

Argument for removing knitr:

• It simplifies the package
• It makes it easier to fulfill the promise of reproducibility, because we are no longer dependent on any packages, other than base R

Argument against removing knitr:

• It removes the capability of searching for library() calls in Rmarkdown in your project
• Breaks backward capability

@piccolbo and @revodavid Any opinions on this?

[andrie]

One further consideration: AFAIK, knitr doesn't play very well with RHEL, due to package dependencies that are not in sync between knitr and RHEL. More precisely, knitr has system dependencies that are not always avilable on the latest RHEL build.

For this reason, I am strongly of the opinion we should remove knitr from checkpoint

[piccolbo]

OK, but what about the other side of the coin? Who was for rmarkdown
support in the first place, what was the argument there? I think we should
have an option not to scan anything by the way and let people install what
they want. In that case we could make knitr a suggest and just issue a
warning when needed but not available. Whoever is doing rmarkdown must have
knitr, so they should be OK, and people who don't don't have the burden of
installing knitr. Anyhow this has to wait for the next rev because of the
size of the change and an incompatibility it introduces.

Antonio

On Sun, Oct 5, 2014 at 8:01 AM, Andrie notifications@github.com wrote:

One further consideration: AFAIK, knitr doesn't play very well with RHEL,
due to package dependencies that are not in sync between knitr and RHEL.
More precisely, knitr has system dependencies that are not always avilable
on the latest RHEL build.

For this reason, I am strongly of the opinion we should remove knitr from
checkpoint

—
Reply to this email directly or view it on GitHub
#137 (comment)
.

[andrie]

I don't remember an debate on whether we should include Rmarkdown or not. The idea (and code) comes from the packrat package and essentially came "for free".

Maybe we should have removed this when we did either bigbang or atilla.

The code change itself is really simple: Simply change the regex to search only for .R and .r files in projectScanPackages() - line 5 of scanRepoPackages.R

I like the idea of potentially turning knitr into a Suggests or Enhances dependency. It's neat, but it comes at a cost - the package will then behave differently for different people. That breaks the big idea of reproducibility, don't you think?

[revodavid]

Can we remove the package dependency on knitr and only do that check on Rmarkdown if knitr is already attached. (That would make knitr an "improves" package, not a dependency.)

[piccolbo]

That would make it a suggests package. we can do that but I wouldn't rush
it into this release because we are supposed to be in bugfix-only mode.

Antonio

On Mon, Oct 6, 2014 at 5:58 AM, David Smith notifications@github.com
wrote:

Can we remove the package dependency on knitr and only do that check on
Rmarkdown if knitr is already attached. (That would make knitr an
"improves" package, not a dependency.)

—
Reply to this email directly or view it on GitHub
#137 (comment)
.

[revodavid]

I think we should remove the dependency on knitr, and advertise Rmarkdown checking as available only when knitr is installed and available. That would make knitr a suggested package for checkpoint, not a dependency.

[piccolbo]

Yep, you just need to locate any knitr-related logic and wrap it in an
if(require(knitr)) statement and maybe issue a warning on the else side.

On Fri, Nov 14, 2014 at 8:59 AM, David Smith notifications@github.com
wrote:

I think we should remove the dependency on knitr, and advertise Rmarkdown
checking as available only when knitr is installed and available. That
would make knitr a suggested package for checkpoint, not a dependency.

—
Reply to this email directly or view it on GitHub
#137 (comment)
.

[andrie]

I have removed this dependency in the branch https://github.com/RevolutionAnalytics/checkpoint/tree/remove_knitr_dependency.

Now need to build additional unit tests that include at least one .Rmd file with required packages.