What does `test_name` equal `NA` mean?

[vgherard]

Hi, I'm trying autotest::autotest_package() on my own package r2r and am getting the following results:

autotest::autotest_package(package = "r2r", test = TRUE)
#> ٭ Extracting example code from 12 .Rd files
#>   |                                                                              |                                                                      |   0%  |                                                                              |======                                                                |   8%  |                                                                              |============                                                          |  17%  |                                                                              |==================                                                    |  25%  |                                                                              |=======================                                               |  33%  |                                                                              |=============================                                         |  42%  |                                                                              |===================================                                   |  50%  |                                                                              |=========================================                             |  58%  |                                                                              |===============================================                       |  67%  |                                                                              |====================================================                  |  75%  |                                                                              |==========================================================            |  83%  |                                                                              |================================================================      |  92%  |                                                                              |======================================================================| 100%
#> ✓ Extracted example code
#> ٭ Converting 10 examples to yaml
#>   |                                                                              |                                                                      |   0%  |                                                                              |=======                                                               |  10%  |                                                                              |==============                                                        |  20%  |                                                                              |=====================                                                 |  30%  |                                                                              |============================                                          |  40%  |                                                                              |===================================                                   |  50%  |                                                                              |==========================================                            |  60%  |                                                                              |=================================================                     |  70%  |                                                                              |========================================================              |  80%  |                                                                              |===============================================================       |  90%  |                                                                              |======================================================================| 100%
#> ✓ Converted examples to yaml
#> 
#> ── autotesting r2r ──
#> 
#> ✓ [1 / 11]: has_key
#> ✓ [2 / 11]: compare_fn
#> ✓ [3 / 11]: default
#> ✓ [4 / 11]: hash_fn
#> ✓ [5 / 11]: hashmap
#> ✓ [6 / 11]: hashset
#> ✓ [7 / 11]: insert
#> ✓ [8 / 11]: keys
#> ✓ [9 / 11]: on_missing_key
#> ✓ [10 / 11]: query
#> ✓ [11 / 11]: values
#> # A tibble: 34 x 9
#>    type   test_name  fn_name parameter parameter_type operation  content   test 
#>    <chr>  <chr>      <chr>   <chr>     <chr>          <chr>      <chr>     <lgl>
#>  1 error  <NA>       hashmap <NA>      <NA>           normal fu… 'hash_fn… TRUE 
#>  2 error  return_su… hashmap (return … (return objec… error fro… 'hash_fn… TRUE 
#>  3 error  <NA>       hashset <NA>      <NA>           normal fu… 'hash_fn… TRUE 
#>  4 error  return_su… hashset (return … (return objec… error fro… 'hash_fn… TRUE 
#>  5 warni… par_match… has_key x         <NA>           Check tha… Paramete… TRUE 
#>  6 warni… par_match… compar… x         <NA>           Check tha… Paramete… TRUE 
#>  7 warni… par_match… default x         <NA>           Check tha… Paramete… TRUE 
#>  8 warni… par_match… hash_fn x         <NA>           Check tha… Paramete… TRUE 
#>  9 warni… par_match… hashmap hash_fn   <NA>           Check tha… Paramete… TRUE 
#> 10 warni… par_match… hashmap compare_… <NA>           Check tha… Paramete… TRUE 
#> # … with 24 more rows, and 1 more variable: yaml_hash <chr>

^{Created on 2021-07-16 by the reprex package (v2.0.0)}

What does the NA test name mean? I've tried to search the documentation and find no special mention.

Thanks, Valerio.

[vgherard]

N.B. above I'm testing the CRAN version v0.1.1 of r2r.

[mpadge]

Thanks @vgherard - that NA happens when a normal function call prompts an error, which in this is this error, generated directly from the example of hashmap:

library (r2r)
rd <- tools::Rd_db ("r2r")
rd <- rd [grep ("^hashtable\\.Rd$", names (rd))]
ex <- tools:::.Rd_get_metadata (rd [[1]], "examples")
print (strsplit (ex, "\n"))
#> [[1]]
#>  [1] "m <- hashmap("                                              
#>  [2] ""                                                           
#>  [3] "        list(\"foo\", 1),"                                  
#>  [4] ""                                                           
#>  [5] "        list(\"bar\", 1:5),"                                
#>  [6] ""                                                           
#>  [7] "        list(data.frame(x = letters, y = LETTERS), \"baz\")"
#>  [8] ""                                                           
#>  [9] "        )"                                                  
#> [10] ""                                                           
#> [11] "m[[ data.frame(x = letters, y = LETTERS) ]]"                
#> [12] ""                                                           
#> [13] ""                                                           
#> [14] ""                                                           
#> [15] "# Set of character keys, case insensitive."                 
#> [16] ""                                                           
#> [17] "s <- hashset(\"A\", \"B\", \"C\", key_preproc = tolower)"   
#> [18] ""                                                           
#> [19] "s[[\"a\"]]"

Running that example exactly as specified is okay:

hashmap (list ("foo", 1),
         list ("bar", 1:5),
         list (data.frame (x = letters, y = LETTERS), "baz"))
#> An r2r hashmap.

These are the parameters of the function:

formals (hashmap)
#> $...
#> 
#> 
#> $hash_fn
#> default_hash_fn
#> 
#> $compare_fn
#> identical
#> 
#> $key_preproc_fn
#> identity
#> 
#> $on_missing_key
#> [1] "default"
#> 
#> $default
#> NULL

And autotest works by submitting the parameters in named forms, the first one of which is okay:

hashmap (... = list ("foo", 1),
         list ("bar", 1:5),
         list (data.frame (x = letters, y = LETTERS), "baz"))
#> An r2r hashmap.

But attempting to name the second parameter triggers this error:

hashmap (... = list ("foo", 1),
         hash_fn = list ("bar", 1:5),
         list (data.frame (x = letters, y = LETTERS), "baz"))
#> Error: 'hash_fn' must be a function.

^{Created on 2021-07-16 by the reprex package (v2.0.0.9000)}

Because an error is triggered by normal usage as shown in the example, there is no named test associated. I'll clarify in this point in the documentation to avoid future confusion. My Q for you: Does this make sense in the context of your r2r package? In essence, what autotest is doing is expecting the order or parameters to always and strictly follow the order of their names, whereas what r2r does is to assume all parameters are part of ... unless they are named, right? So all of those parameters in the example are then dumped together in your validate_hashmap_args() fn as a list() object.

This in essence comes down to an unwritten convention that ... parameters are expected to come at the end of function inputs, and not at the start as done here. Note, however, that this is just something that appears to be sufficiently common practice, for example in the tidyeval description of "...", in which all examples place them at the end, as do (almost?) all base and stats fns. As Hadley says here:

Using ... comes with [a] two downsides:

When you use it to pass arguments to another function, you have to carefully explain to the user where those arguments go.

Neither autotest, nor our general standards, have a policy on this at this stage, but this case clearly suggests some need for such. A solution is, however, only likely to be to require that .. always come as the last parameter. That would mess with some of the functionality of r2r though, right? Feedback appreciated! Thanks!

[vgherard]

Thanks @mpadge for the detailed reply, I see the problem.

In essence, what autotest is doing is expecting the order or parameters to always and strictly follow the order of their names, whereas what r2r does is to assume all parameters are part of ... unless they are named, right?

It looks exactly like this. In my package, this is purely intentional: calls to hashmap() without any named argument should only be used to populate the hash table with some initial elements, whereas parameters should be named explicitly.

Neither autotest, nor our general standards, have a policy on this at this stage, but this case clearly suggests some need for such. A solution is, however, only likely to be to require that .. always come as the last parameter. That would mess with some of the functionality of r2r though, right? Feedback appreciated! Thanks!

The general rule of thumb makes a lot of sense, for sure. However, I think the present use case in r2r described above makes the case for a justified exception (placing the ... at the end, I would need to specify all arguments in order to populate my hashtable...). No idea of how complicated this would be, but maybe you could check whether the ellipsis argument is the last one before performing some tests?

Thanks, V

[mpadge]

No idea of how complicated this would be, but maybe you could check whether the ellipsis argument is the last one before performing some tests?

Yes, indeed, that could be done. The entire parsing routines are going to be restructured, or more accurately removed to be replaced by a much more efficient approach to identifying input types of each parameter. That will change a lot of this anyway, so I'll first wait for that before returning to this point, but will definitely leave it open to address it asap. Thanks!