What to do about .. paths that *are* in the project directory

[beanumber]

Suppose I am in a subdirectory of the project. Then ../data.csv could be a relative path to a file that exists within the project. Should this generate an error?

[beanumber]

It's actually worse than that. If here::here() returns /path/to/dir, then consider:

is_path_here("../dir/data.csv")

Currently this returns TRUE, because that is the behavior of fs::path_has_parent(). But for reproducibility's sake it seems like it should return FALSE, because dir isn't necessarily the name of the directory on someone else's computer.

What do you think @jennybc @hadley ?

[jennybc]

I would seriously consider requiring the use of here for your package to do its thing. I honestly think these practices of writing paths with lots of .. are a workaround for a problem that here solves more properly.

[beanumber]

It does require here! [But maybe not in the way you are imagining.]

My question is more philosophical, I think. If I reference a file that is in the project directory, but the path that I use to that file references places outside the project directory, then is that a bad path?

I feel like the answer is "yes", but the current implementation reflects "no."

[beanumber]

An even more subtle distinction:

1. A file located in here("R") references a file at ../data/data.csv
2. A file located here() references a file at ../data/data.csv
3. A file located here() references a file at ../data/data.csv, but coincidentally the working directory is also called data

Which should throw errors? [My feeling is 2 and 3, but not 1. Currently only 2 throws an error.]

In any case, perhaps @jennybc wants to suggest that in all three cases people write the path as here("data", "data.csv")? We can make that suggestion...

[jennybc]

In any case, perhaps @jennybc wants to suggest that in all three cases people write the path as here("data", "data.csv")? We can make that suggestion...

Yeah, that is definitely my point.

If a file located in here("R") references a file via ../data/data.csv, that implies execution with working directory set to the R/ directory, right?

In any case, I suppose your analysis of file paths shouldn't throw errors but should call out dangerous practices. And using up references like .. is a giant red flag re: undocumented assumptions re: working directory.

[beanumber]

Right. So one question is whether we should do some kind of regexp search for paths that contain .., or just rely on fs::path_has_parent() and other path computation tools. @hadley prefers the latter, but maybe we need both.

Also giving me fits is the fact that running my tests

1. directly in the console
2. using devtools::test()
3. using devtools::check()

Results in three different values of here::here(). I guess ultimately this is good, but my head is spinning!

[jennybc]

I don't understand why someone using here() would ever write a path that contains ... Do you think there's a legitimate need for that? It feels like one of these half-hearted, partial implementation things, where someone technically uses here ... but then still uses their previous workarounds for writing relative paths, assuming manual manipulation of working directory. Like starting to use git but also continuing to save new versions of a file with a new name.

Re: here::here():

I travelled this ground pretty recently with Irene (intern) and a lesson she learned .. is that you should never use here in a package. I understand that you are assessing user code that uses here. But in your package code, you should almost certainly be using rprojroot. here::here() determines the root directory at the time of the first call and caches it, which might be what you're wrestling with. You probably want discovery of project root at the instant of the call. You might need to do something more like this:

https://github.com/r-lib/usethis/blob/1e3c6a66e8b2d2790ee6d7e6d5651c52fb61abfc/R/proj.R#L169-L174

In terms of testing infrastructure, you might enjoy something like the scoped_temporary_*() helpers in usethis. Getting those helpers set up well for interactive (test) development and arm's length use via test() and check() was a major improvement to my quality of life and s/w quality.

https://github.com/r-lib/usethis/blob/1e3c6a66e8b2d2790ee6d7e6d5651c52fb61abfc/tests/testthat/helper.R#L8-L52