cbeleites / hyperSpec

R package hyperSpec can now be found at https://github.com/r-hyperspec/hyperSpec
http://r-hyperspec.github.io/hyperSpec/
GNU General Public License v3.0
22 stars 11 forks source link

Shorten the list of dependencies #215

Open GegznaV opened 4 years ago

GegznaV commented 4 years ago

The list of dependency packages in DESCRIPTION should be shortened as some fo those packages are not used in hyperSpec anymore.

cbeleites commented 4 years ago

IMHO this should include possibly putting vignettes into .Rbuildignoe if that allows us to get rid of dependencies/suggestions.

We'd still have them built in CI (via our separate roxygenize() step), and e.g. included in the pkgdown articles.

But our own CI failing due to a breaking change in the dependency isn't nearly as bad as an ultimatum by CRAN...

GegznaV commented 4 years ago

I'm not sure if it is a good idea to .Rbuildignore vignettes especially in situations, when a user is offline. It is better to restructure the code in vignettes, disable evaluation, or move some non-essential sections to a separate vignette and ignore only that one.

@bryanhanson Have we removed/commented some sections with additional dependencies until now in the hyperSpec vignette (or elsewhere)? Do you have any ideas what could be done regarding this issue?

bryanhanson commented 4 years ago

I have only worked on hyperSpec.Rmd and it does not have sections conditionally disabled.

On the main question of this issue, shouldn't the pkgs listed in DESCRIPTION Suggests: or Depends: that are no longer needed in hyperSpec because functions have moved to other packages move to the other pkgs, and then be deleted from hyperSpec? Same idea as deprecated functions and unit tests that have to move. Some of the Suggests: may be exclusively due to their use in vignettes, and some vignettes will move and allow the removal from hyperSpec.

Back to the vignette aspect: one can certainly condition a code chunk based on package availability, but there's no way to know if a given package is available but broken.

cbeleites commented 4 years ago

As for the vignettes, I was thinking along the lines that sure sessioninfo::session_info() looks nicer than plain sessionInfo() - but is that actually worth another dependency? (To me, it is not)

cbeleites commented 4 years ago

@GegznaV the rgl plots in the laser vignette are conditional on rgl being available.

GegznaV commented 4 years ago

On sessioninfo::session_info() is not a heavy dependency, R core team is in the list of authors/contributors (so I don't think it will break easily) and it is easy to condition not to use the function if the package is not installed. So I prefer to keep sessioninfo as a suggested dependency.


On rgl. I think, that I disabled all code that uses rgl. It is a heavy dependency and there were plenty of OS-specific issues related to automatic deployment and rgl. So I prefer to get rid of rgl as a dependency.

GegznaV commented 4 years ago

When I opened this issue, I was mainly thinking of removing R.rsp package.

cbeleites commented 4 years ago

R core team is in the list of authors/contributors (so I don't think it will break easily)

that's what I thought with the package XML as well - but beginning of the year it was gone from CRAN sufficiently long to also have hyperSpec being thrown out...


Showcases

Wrt. rgl: I'd still like to show in a vignette/article that if people get it installed, they can use it with hyperSpec. There are other packages that are also needed only as "showcases" (akima) - but IMHO such showcases are one of the main purposes of vignettes/articles.

My thoughts are:

We can still have the CI checking that the vignettes can actually be built - so we know when something breaks. But we then don't have the pressure to fix & submit to CRAN within their deadlines.


UX/UI

Have a look at #218 - I implemented a function attach_hySpc() to load most of the hySpc.* package zoo that is installed.

cbeleites commented 4 years ago

@bryanhanson deleting dependencies when functionality moves: of course, and (at least for proper dependencies) R CMD check will also tell us if we list dependencies that are never actually used.

I'm thinking a bit higher-level here: is there functionality that we import that is not worth the "exposure"?

GegznaV commented 4 years ago

I'm OK with shipping pre-built vignettes (with R.rsp or otherwise)

knitr is used to build HTML vignettes now. I do not understand what you have in mind @cbeleites. The only place I find R.rsp mentioned in the package now is the DESCRIPTION file.

that's what I thought with the package XML as well

In which category of dependencies XML was: Depends, Includes, or Suggests? As I can infer from the position of xml2, it was in Depends. It is critically important that packages in Depends and Includes would be reliable and stable. And the story is different for Suggests as these packages are optional.

such showcases are one of the main purposes of vignettes/articles.

Then could there be a separate vignette for those showcases?

cbeleites commented 4 years ago

Wrt. R.rsp: it provides vignette engine "as.is" which includes prebuilt vignettes, i.e. we build the vignette locally/on CI but R CMD check or the CRAN checks do not build it again. That is, the package will not need to even suggest any particular packages needed to build that vignette. The vignette document (pdf/html) will be available offline for the users, though.

Even though packages in Suggests are optional, CRAN checks include building all vignettes, and if that fails (e.g. because a suggested package changes/becomes unavailable/...) hyperSpec will have an error as well, and we're back in the situation with the deadline.

Depends/Includes vs. Suggests is mostly a difference on user side (you don't have to install suggested packages), but not so much on CRAN/developer side.

GegznaV commented 4 years ago

Wrt. R.rsp: it provides vignette engine "as.is" which includes prebuilt vignettes, i.e. we build the vignette locally/on CI but R CMD check or the CRAN checks do not build it again. That is, the package will not need to even suggest any particular packages needed to build that vignette. The vignette document (pdf/html) will be available offline for the users, though.

To achieve this, is it enough to have R.rsp in Suggests only and nowhere else in the package mentioned? Understand, that this was useful when PDF vignettes and especially make were used. But I can't understand, how it could be useful for HTML vignettes with knitr as build engine.


About the other dependencies: I think, we'll discuss this on Monday.

GegznaV commented 4 years ago

Make things (dependencies) simpler

GegznaV commented 4 years ago

Make things (dependencies) simpler