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

load_all and system.file #179

Closed
ramnathv opened this Issue Oct 29, 2012 · 6 comments

Comments

Projects
None yet
4 participants
@ramnathv
Copy link

ramnathv commented Oct 29, 2012

I am having this problem with a package that I am developing. I use system.file to refer to external assets in the package directory. When I dont use load_all things work fine. But, when I use load_all, system.file is no longer able to detect package assets and as a result the code crashes.

I can provide a reproducible example, but it will involve downloading and installing a package under development. Let me know.

@wch

This comment has been minimized.

Copy link
Member

wch commented Oct 30, 2012

See the inst_path() function from staticdocs for a workaround:
https://github.com/hadley/staticdocs/blob/master/R/util.r

The root of the problem is that the directory structure of an installed package is different from a development package.

@ramnathv

This comment has been minimized.

Copy link
Author

ramnathv commented Nov 1, 2012

Thanks. I will check that.

@ramnathv ramnathv closed this Nov 1, 2012

@bbolker

This comment has been minimized.

Copy link
Contributor

bbolker commented Nov 7, 2012

sorry to be a bonehead, but I'm just getting started with devtools so may be a little obtuse. I am having this precise problem. I can follow the link to the inst_path function, but I'm not sure how I can use to it to work around the problem ... hints? Seems related to #74

@hadley hadley reopened this Nov 7, 2012

@hadley

This comment has been minimized.

Copy link
Member

hadley commented Dec 12, 2012

@wch I think we need an environment of functions that need to be patched for devtools loaded packages - these would always be copied into the package namespace, and would include system.file to start with.

@wch

This comment has been minimized.

Copy link
Member

wch commented Dec 12, 2012

OK, this could be tricky... I don't think it's possible to make a 100% compatible version of system.file.

Here's one possible way to do it:

Suppose that the name of the development package is "newpackage", and the path is /foo/newpackage.

  • If called with system.file(package="stats"), where stats is any package other than newpackage, just pass through to base::system.file.
  • If called with system.file(package="mypackage") have some customized behavior:
    • If the path requested is /, return /foo/newpackage.
    • If the path requested is /DESCRIPTION, /NEWS, or /NAMESPACE, return /foo/newpackage/xxx, where xxx is the file.
    • If the path is data/, then return /foo/newpackage/data/
    • If the path is anything else, return the path under /foo/newpackage/inst/

Limitations:

  • Any calls to system.file(package="newpackage") from other packages or from the R console will get base::system.file, not the modified version.
  • If users manually get a subdir based off the top level of newpackage, it'll probably be wrong. For example, file.path(system.file(package="newpackage"), "somefile.txt") will return /foo/newpackage/somefile.txt, instead of /foo/newpackage/inst/somefile.txt, which is what they probably want.
  • If functions like find.package and path.package are used, they still won't quite work right (this is almost the same as previous). For example, if someone does something like file.path(find.package("newpackage"), "somefile.txt"), it'll return /foo/newpackage/somefile.txt, instead of /foo/newpackage/inst/somefile.txt, which is what they probably want.
  • Tangential: other functions that use lib.loc or libpath won't work correctly, and I don't think it's possible to make them work correctly. For example, see #191.

So, overall, it might be possible to make a version of system.file that sort of works, but there will be a lot exceptions to the behavior, which is why I'm kind of lukewarm on it. The fundamental problem is that the directory structure of an installed package is different from the directory structure of an in-development package.

@hadley

This comment has been minimized.

Copy link
Member

hadley commented Dec 12, 2012

Since system.file only returns a path if the file exists, I think you can make the algorithm a little simpler. Just look in look in "/inst/x" and if it's not in there look in "/x".

Your other objections:

  • I don't care about find.package or path.package - system.file is the most commonly used
  • It only matters for functions in the development package, otherwise system.file should point to the installed locatio
  • We haven't encountered problems with lib.loc /libpath yet, so let's not worry about them ;)

@wch wch closed this in be1017c Dec 14, 2012

@lock lock bot locked and limited conversation to collaborators Sep 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.