On 02/08/2016 4:20 PM, Dirk Eddelbuettel wrote:

On 2 August 2016 at 16:07, Duncan Murdoch wrote:
| On 02/08/2016 1:41 PM, Dirk Eddelbuettel wrote:
| >
| > On 2 August 2016 at 13:12, Duncan Murdoch wrote:
| > | On 02/08/2016 1:01 PM, Dirk Eddelbuettel wrote:
| > | >
| > | > On 2 August 2016 at 11:36, Joshua Ulrich wrote:
| > | > | Maybe I'm missing something, but isn't that the point of calling
| > | > | requireNamespace()?  For example:
| > | > | if (requireNamespace("suggestedPackage"))
| > | > |   stop("suggestedPackage required but not installed")
| > | > |
| > | > | That doesn't seem like a heavy burden for package writers when writing
| > | > | infrequently used functions in their package.  You could even add the
| > | > | install.packages command to the error message to instruct users to
| > | > | install the required package.
| > | >
| > | > [...]
| > | >
| > | > | I personally would not want install.packages() to install packages I'm
| > | > | unlikely to actually use.
| > | > |
| > | > | It's also not clear to me how importing rarely used functions causes
| > | > | bloat, but installing all packages for all rarely-used functions does
| > | > | not cause bloat.
| > | >
| > | > Sadly, some people whose advocacy is taken as religous gospel in some
| > | > circles, particularly beginners, advocate pretty much that: treat 
Suggests:
| > | > as Depends: and install it anyway because, hell, why would one tests.
| > | >
| > | > I regularly run (large) reverse depends checks against some of my more 
widely
| > | > used packages and run into this all the time.
| > | >
| > | > We (as a community, including CRAN whose gatekeepers I have failed to
| > | > convince about this on on multiple attempts) are doing this wrong.
| > | > "Suggests:" really means optionally, rather than unconditionally.  But 
it
| > | > would appear that you and I are about to only ones desiring that 
behaviour.
| > |
| > | I thought I understood Joshua's point (and agreed with it), but you also
| > | seem to be agreeing with him and I don't understand at all what you're
| > | saying.
| > |
| > | What is "this" in your last paragraph, that you have failed to convince
| > | CRAN gatekeepers about?
| >
| > It is really simple:  Having _both_ Suggests: foo _and_ an unconditonal call
| > to foo::bar() in the code.
| >
| > Whereas Josh and I argue that it needs to be conditional on requireNames()
| > coming back TRUE.
|
| I am feeling particularly dense today.  What about "Having _both_
| Suggests: foo _and_ an unconditonal call to foo::bar() in the code." did
| you fail to convince CRAN about?  That it is a good thing, or a bad thing?

It is a Really Bad Thing )TM) because I read Writing R Extensions as meaning
that these ought to be tested for presence (as Suggests != Depends, so we
cannot [rather: should not] assume them) yet package authors are sloppy (and
CRAN let's them, being charged as facilitator to a crime here) and BAM I get
a failing test as some bad, bad code unconditionally calls code that should
only be called conditional on a suggested package actuallt being there.

Okay, now I think I understand, but I agree with CRAN. It is not feasible to tell if the test happened somewhere in the code unless we enforce a particular way of writing the test.

I would object if I had to write if (requireNamespace("foo")) multiple times just to satisfy CRAN's test, when any sane human could tell that the first test was sufficient.

For example, if my package Suggests: foo, I should be able to write

if (!requireNamespace("foo"))
  stop("Package foo is needed for this example")

and then merrily call foo::bar() as many times as I like.

Or am I still misunderstanding you? What particular thing should CRAN change?

Duncan Murdoch

______________________________________________
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel

Reply via email to