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

I want to depend on littler, please export something #40

Closed
gaborcsardi opened this issue Jul 6, 2016 · 15 comments
Closed

I want to depend on littler, please export something #40

gaborcsardi opened this issue Jul 6, 2016 · 15 comments

Comments

@gaborcsardi
Copy link

@gaborcsardi gaborcsardi commented Jul 6, 2016

I want to use littler from my package, but I don't want to Depend on it, because of the lengthy startup message. I think the message is useful in general, but maybe not so much if littler is just the background machinery of another package.

Then the problem is that I cannot use Imports, either, because then I need to import sg to avoid an R CMD check warning. But there is nothing to import.

I quite like the fact that littler does not export anything. But I think it might be worth exporting a little something so that I can Import the package cleanly. E.g. how about importing a single variable or function r which just points to the binary of littler? That's even (slightly) useful, I don't need to call system.file(), just use littler::r.

Alternative suggestions are also welcome.

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jul 7, 2016

What message?

edd@don:~$ r -e 'q()'
edd@don:~$ r -e 'cat("Hi Gabor\n"); q()'
Hi Gabor
edd@don:~$ 
@gaborcsardi
Copy link
Author

@gaborcsardi gaborcsardi commented Jul 7, 2016

I want to use it from an R package. Then if one loads my package, one gets

Loading expect
Loading required package: littler
The littler package provides 'r' as a binary.
On OS X, 'r' and 'R' are the same so 'lr' is an alternate name for littler.

because I need Depends...

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jul 7, 2016

Dunno. Can you not suppressPackageStartupMessage() when Importing? Seems like an R bug.

@gaborcsardi
Copy link
Author

@gaborcsardi gaborcsardi commented Jul 7, 2016

I depend on your package, so I don't think I can specify suppressPackageStartupMessage.....

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jul 7, 2016

Try suppressMessages(library(myPackageInQuestion)) which is what I do.

Else, I suppose, I could be silent if getOption("littlerPleaseBeSilent", FALSE) returns TRUE ie when the option is set. But all this is a wee bit orthogonal to the normal usage of the package ... (but new to us now that the package is in fact on CRAN).

@gaborcsardi
Copy link
Author

@gaborcsardi gaborcsardi commented Jul 7, 2016

I can't do suppressMessages(library(myPackageInQuestion)) because I don't call library() at all. I have:

...
URL: https://github.com/mangothecat/expect
BugReports: https://github.com/mangothecat/expect/issues
Depends:
    littler
Imports:
    R6
...

An option would be great!

I think this is a bug in R CMD check. An R package can be useful and required, even if one does not import anything from it. One can still use the files it supplies....

I suppose this is not the regular littler usage, but it is indeed one of the "benefits" of having it in a CRAN package. :)

@gaborcsardi
Copy link
Author

@gaborcsardi gaborcsardi commented Jul 7, 2016

I am not sure that an option would work, actually. littler is loaded before my package is loaded, so I might not have time to set the option. Maybe in .onLoad.

@gaborcsardi
Copy link
Author

@gaborcsardi gaborcsardi commented Jul 7, 2016

Btw it looks like to find the executable from R, I need sg like

file.path(system.file(package="littler"), "bin",
                   paste0("r", ifelse(.Platform$OS.type=="windows", ".exe", "")))

(from littler itself).

Maybe it makes sense to provide this as an r() function, because it is quite error prone to write it?

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jul 7, 2016

Maybe. I can look at what tools and utils do.

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jul 7, 2016

BTW there is no .Platform$OS.type=="windows". It is "OSs with an X only".

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jul 7, 2016

So this, I guess, it close:

R> littler:::test("cat(system.file(package='littler', 'bin', 'r'))")
[1] "/usr/local/lib/R/site-library/littler/bin/r"
R> 
@gaborcsardi
Copy link
Author

@gaborcsardi gaborcsardi commented Jul 7, 2016

Well, I took the code from liittler, and I assumed that it was right. :)
https://github.com/eddelbuettel/littler/blob/738371378d4e03f27adb3b01e38ecd5ea4873c2a/R/test.R#L2-3

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jul 7, 2016

Yes, I later read up on test.r -- not something I wrote initially, but simply accommodating a PR by somebody else.

@gaborcsardi
Copy link
Author

@gaborcsardi gaborcsardi commented Aug 6, 2016

Thanks! Btw. I just found out that if you need to import littler, you can also just import the whole package, and apparently R CMD check is fine with that (even if nothing will be imported).

I still think that an r() function makes sense, but it seems that you don't actually need it to be able to import the package. FYI.

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Aug 6, 2016

Well...now it is in there. It just a one-liner, really, and a few lines of roxygen.

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.