Unexpected behavior from has_dirname() criterion

[jonathan-g]

I am getting unexpected behavior from has_dirname as a criterion:

Reproducible example

Consider this reprex:

if (!dir.exists("tmp")) dir.create("tmp")
if (!dir.exists("tmp/foo")) dir.create("tmp/foo")
if (!dir.exists("tmp/boohoo")) dir.create("tmp/boohoo")
if (!dir.exists("tmp/foo/bar")) dir.create("tmp/foo/bar")

crit <- rprojroot::has_dirname("foo")
p <- rprojroot::find_root(criterion = crit, path = "tmp/foo/bar")
p
#> [1] "C:/Users/<username>/AppData/Local/Temp/RtmpmmXvM3/reprex52509e82288/tmp/foo"
crit$testfun[[1]]("tmp/foo")
#> [1] TRUE
paste(p, "is the root because ", rprojroot::get_root_desc(crit, p))
#> [1] "C:/Users/<username>/AppData/Local/Temp/RtmpmmXvM3/reprex52509e82288/tmp/foo is the root because  directory name is `foo`"

crit <- rprojroot::has_dirname("boohoo")
p <- rprojroot::find_root(criterion = crit, path = "tmp/foo/bar")
p
#> [1] "C:/Users/<username>/AppData/Local/Temp/RtmpmmXvM3/reprex52509e82288/tmp/foo"
crit$testfun[[1]]("tmp/foo")
#> [1] TRUE
paste(p, "is the root because ", rprojroot::get_root_desc(crit, p))
#> [1] "C:/Users/<username>/AppData/Local/Temp/RtmpmmXvM3/reprex52509e82288/tmp/foo is the root because  directory name is `boohoo`"

unlink(c("tmp/foo/bar", "tmp/foo", "tmp/boohoo", "tmp"))

^{Created on 2019-12-26 by the reprex package (v0.3.0)}

Expected behavior

The documentation says:

The has_dirname() function constructs a criterion that checks if the base::dirname() has a specific name

This makes it sound as though the function is checking whether the parent node of the candidate path (base::dirname(path)) has a given name.

On the other hand, get_root_desc() says that ./tmp/foo is a root because "directory name is boohoo", which makes it sound like the criterion is that the leaf node of the candidate path (base::basename(path)) has a given name.

The documentation and the output from get_root_desc are confusing and seem inconsistent with each other.

But in either case, based on the documentation and the output from get_root_desc, I would expect find_root(crit, "tmp/foo/bar") to fail with an error message when crit is has_dirname("boohoo") and for crit.testfun[1]("tmp/foo") to return FALSE.

Actual behavior

However, the criterion really checks whether the parent node has a child with the given name.

This means that a directory can fulfill the criterion if it has a sibling with the specified name, as in the example above: tmp/foo and tmp/boohoo are siblings, and tmp/foo satisfies the has_dirname("boohoo") criterion and find_root(has_dirname("boohoo"), "tmp/foo/bar") succeeds and returns tmp/foo instead of failing, as I would expect, and has_dirname("boohoo").testfun[1]("tmp/foo") returns TRUE instead of the expected FALSE

Possible remedies:

Update Documentation for has_dirname()

If this is the desired behavior, then it would be good to make the documentation clearer, and perhaps provide an example to illustrate the behavior.

Change behavior of has_dirname()

If it is not the desired behavior, then changing the testfun from

function(path) {
    dir.exists(file.path(dirname(path), "boohoo"))
}

to something like

function(path) {
  basename(dirname(path)) == "boohoo"
}

With this latter function, tmp/foo/bar would satisfy has_dirname("foo") and tmp/foo and tmp/boohoo would both satisfy has_dirname("tmp"), but tmp/foo would not satisfy has_dirname("boohoo") and tmp/boohoo would not satisfy has_dirname("foo").

Alternately (another possible interpretation of the intended behavior) would be

function(path) {
  dir.exists(path) && basename(path) == "boohoo"
}

In this case, tmp/foo would satisfy has_dirname("foo") and tmp/boohoo would satisfy has_dirname("boohoo"), but tmp/foo would not satisfy has_dirname("boohoo").

Session Info

R version 3.6.2 (2019-12-12)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 18363)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.1252  LC_CTYPE=English_United States.1252    LC_MONETARY=English_United States.1252
[4] LC_NUMERIC=C                           LC_TIME=English_United States.1252    

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] rprojroot_1.3-2

loaded via a namespace (and not attached):
[1] compiler_3.6.2  backports_1.1.5 tools_3.6.2     packrat_0.5.0  

[krlmlr]

Thanks for the detailed description.

I think I confused dirname() with basename() when implementing this. It makes more sense to me to think of a criterion that defines that the basename of a directory matches the input, give or take case sensitivity of the file system. Can you confirm?

[krlmlr]

Scratch that, I now better understand the problem. PR coming.

[github-actions]

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.