brolgar submission #1

mpadge · 2020-09-23T15:13:23Z

Thanks @njtierney for volunteering to submit your brolgar package for trial "soft submission" to rOpenSci's new system for peer review of statistical software. This issue includes output from our automated assement and reporting tools developed as part of this new system. These currently function as the following two distinct components (which will be integrated later):

packgraph

library (packgraph)
packageVersion ("packgraph")
#> [1] '0.0.0.8'
package <- "/<local>/<path>/<to>/fastadi"

g <- pg_graph (package, plot = FALSE)
pg_report (g)
#> ══ brolgar ═════════════════════════════════════════════════════════════════════
#> 
#> The package has 48 exported functions, and 17 non-exported funtions. The
#> exported functions are structured into the following 2 primary clusters
#> containing 6 and 32 functions
#> 
#> 
#> | cluster|  n|name             | num_params| num_doc_words| num_doc_lines| num_example_lines| centrality|
#> |-------:|--:|:----------------|----------:|-------------:|-------------:|-----------------:|----------:|
#> |       1|  1|sample_n_keys    |         NA|            NA|             9|                NA|          2|
#> |       1|  2|add_key_slope    |         NA|            NA|             2|                NA|         NA|
#> |       1|  3|add_n_obs        |          2|           138|             2|                 6|         NA|
#> |       1|  4|key_slope        |          3|           198|             1|                 1|         NA|
#> |       1|  5|sample_frac_keys |         NA|            NA|             2|                NA|         NA|
#> |       1|  6|stratify_keys    |          5|           312|             2|                37|         NA|
#> 
#> 
#> | cluster|  n|name              | num_params| num_doc_words| num_doc_lines| num_example_lines| centrality|
#> |-------:|--:|:-----------------|----------:|-------------:|-------------:|-----------------:|----------:|
#> |       2|  1|b_diff_max        |         NA|            NA|            18|                NA|          1|
#> |       2|  2|b_diff_min        |         NA|            NA|             2|                NA|          1|
#> |       2|  3|b_diff_q25        |         NA|            NA|             2|                NA|          1|
#> |       2|  4|b_diff_q75        |         NA|            NA|             2|                NA|          1|
#> |       2|  5|b_range_diff      |         NA|            NA|            21|                NA|          1|
#> |       2|  6|b_diff_mean       |         NA|            NA|             2|                NA|         NA|
#> |       2|  7|b_diff_median     |         NA|            NA|             1|                NA|         NA|
#> |       2|  8|b_diff_sd         |         NA|            NA|             2|                NA|         NA|
#> |       2|  9|b_diff_var        |         NA|            NA|             2|                NA|         NA|
#> |       2| 10|b_iqr             |         NA|            NA|             2|                NA|         NA|
#> |       2| 11|b_mad             |         NA|            NA|             2|                NA|         NA|
#> |       2| 12|b_max             |         NA|            NA|             2|                NA|         NA|
#> |       2| 13|b_mean            |         NA|            NA|             2|                NA|         NA|
#> |       2| 14|b_median          |         NA|            NA|             2|                NA|         NA|
#> |       2| 15|b_min             |         NA|            NA|             2|                NA|         NA|
#> |       2| 16|b_q25             |         NA|            NA|             2|                NA|         NA|
#> |       2| 17|b_q75             |         NA|            NA|             2|                NA|         NA|
#> |       2| 18|b_range           |         NA|            NA|             2|                NA|         NA|
#> |       2| 19|b_sd              |         NA|            NA|            18|                NA|         NA|
#> |       2| 20|b_var             |         NA|            NA|             2|                NA|         NA|
#> |       2| 21|decreasing        |         NA|            NA|             2|                NA|         NA|
#> |       2| 22|feat_brolgar      |         NA|            NA|            18|                NA|         NA|
#> |       2| 23|feat_diff_summary |         NA|            NA|            56|                NA|         NA|
#> |       2| 24|feat_five_num     |         NA|            NA|             2|                NA|         NA|
#> |       2| 25|feat_monotonic    |         NA|            NA|             2|                NA|         NA|
#> |       2| 26|feat_ranges       |         NA|            NA|            24|                NA|         NA|
#> |       2| 27|feat_spread       |         NA|            NA|             0|                NA|         NA|
#> |       2| 28|feat_three_num    |         NA|            NA|             2|                NA|         NA|
#> |       2| 29|increasing        |         NA|            NA|            18|                NA|         NA|
#> |       2| 30|index_regular     |          2|            62|             0|                 1|         NA|
#> |       2| 31|monotonic         |          2|           314|             0|                58|         NA|
#> |       2| 32|unvarying         |         NA|            NA|            10|                NA|         NA|
#> 
#> There are also 10 isolated functions:
#> 
#> 
#> |  n|name           | loc|
#> |--:|:--------------|---:|
#> |  1|facet_sample   |  24|
#> |  2|facet_strata   |  26|
#> |  3|index_summary  |   8|
#> |  4|keys_near      |   5|
#> |  5|n_obs          |  11|
#> |  6|near_between   |   7|
#> |  7|near_middle    |  11|
#> |  8|near_quantile  |  14|
#> |  9|nearest_lgl    |   5|
#> | 10|nearest_qt_lgl |   6|
#> 
#> ── Documentation of non-exported functions ─────────────────────────────────────
#> 
#> 
#> |value  | doclines| cmtlines|
#> |:------|--------:|--------:|
#> |mean   |     13.4|        0|
#> |median |      2.0|        0|
#> Error in if (r2_ex < 0) out <- c(out, paste0("More central functions should have ", : missing value where TRUE/FALSE needed

autotest

library (autotest)
packageVersion ("autotest")
#> [1] '0.0.1.252'
x <- autotest_package (package)
#> Loading brolgar
#> 
#> ✔ [1 / 8]: add_n_obs
#> ✔ [2 / 8]: index_regular
#> ✔ [3 / 8]: key_slope
#> ✔ [4 / 8]: keys_near
#> ✔ [5 / 8]: monotonic
#> ✔ [6 / 8]: n_obs
#> ✔ [7 / 8]: near_middle
#> ✔ [8 / 8]: near_quantile
x$yaml_hash <- substring (x$yaml_hash, 1, 6)
knitr::kable (x, row.names = TRUE)

	type	fn_name	parameter	operation	content	yaml_hash
1	error	add_n_obs	.data	tabular as data.frame	Input must inherit from tsibble	82ea37
2	error	add_n_obs	.data	tabular as tibble	Input must inherit from tsibble	82ea37
3	error	add_n_obs	.data	tabular as data.table	Input must inherit from tsibble	82ea37
4	diagnostic	add_n_obs	.data	tabular structure with new class structure	Function [add_n_obs] should error when class structure of `data.frame` input is removed.	82ea37
5	diagnostic	add_n_obs	NA	compare class of return value with description	Function returns an object of class [data.frame] yet documentation describes class of value as [dataframe]	82ea37
6	diagnostic	add_n_obs	NA	compare class of return value with description	Function returns an object of primary class [tbl_ts] yet documentation says value is of class [dataframe]	82ea37
7	diagnostic	index_regular	data	tabular structure with new class structure	Function [index_regular] should error when class structure of `data.frame` input is removed.	185a7b
8	error	key_slope	.data	tabular as data.frame	no applicable method for ‘key_slope’ applied to an object of class “data.frame”	9f17ff
9	error	key_slope	.data	tabular as tibble	no applicable method for ‘key_slope’ applied to an object of class “c(‘tbl_df’, ‘tbl’, ‘data.frame’)”	9f17ff
10	error	key_slope	.data	tabular as data.table	no applicable method for ‘key_slope’ applied to an object of class “c(‘data.table’, ‘data.frame’)”	9f17ff
11	diagnostic	key_slope	.data	tabular structure with new class structure	Function [key_slope] should error when class structure of `data.frame` input is removed.	9f17ff
12	error	key_slope	formula	(unquoted) formula param as (quoted) character	:1:1: unexpected ‘<’	9f17ff
13	message	key_slope	formula	(unquoted) formula param as (quoted) character	1: <	9f17ff
14	message	key_slope	formula	(unquoted) formula param as (quoted) character	^	9f17ff
15	diagnostic	key_slope	NA	compare class of return value with description	Function returns an object of primary class [tbl_df] yet documentation says value is of class [data.frame]	9f17ff
16	error	keys_near	.data	tabular as data.frame	object ‘height_cm’ not found	13f36a
17	error	keys_near	.data	tabular as tibble	object ‘height_cm’ not found	13f36a
18	error	keys_near	.data	tabular as data.table	object ‘height_cm’ not found	13f36a
19	error	keys_near	.data	rectangular parameter with extended class structure	object ‘height_cm’ not found	13f36a
20	diagnostic	keys_near	.data	tabular structure with new class structure	Function [keys_near] should error when class structure of `data.frame` input is removed.	13f36a
21	error	keys_near	NA	normal function call	object ‘height_cm’ not found	13f36a
22	error	keys_near	NA	error from normal operation	object ‘height_cm’ not found	13f36a
23	diagnostic	monotonic	x	convert vector input to list-columns	Function [monotonic] errors on list-columns when submitted as x Error message: -(length(r) - lag + 1L)]: non-numeric argument to binary operator	44c892
24	diagnostic	n_obs	x	tabular structure with new class structure	Function [n_obs] should error when class structure of `data.frame` input is removed.	3853b0
25	diagnostic	n_obs	names	substitute integer values for logical parameter	Parameter names of function [n_obs] is assumed to be logical, but responds to general integer values.	3853b0
26	diagnostic	n_obs	names	length 2 vector for single-length parameter	parameter [names] is assumed to be a single value of logical type, yet admits vectors of length > 1	3853b0
27	diagnostic	near_middle	x	convert vector input to list-columns	Function [near_middle] errors on list-columns when submitted as x Error message: comparison of these types is not implemented	e4312f
28	diagnostic	near_quantile	x	convert vector input to list-columns	Function [near_quantile] errors on list-columns when submitted as x Error message: comparison of these types is not implemented	3bedbe
29	diagnostic	near_quantile	probs	custom class definitions for vector input	Function [near_quantile] errors on vector columns with different classes when submitted as probs Error message: no applicable method for ‘round_any’ applied to an object of class “different”	3bedbe
30	diagnostic	near_quantile	probs	convert vector input to list-columns	Function [near_quantile] errors on list-columns when submitted as probs Error message: no applicable method for ‘round_any’ applied to an object of class “AsIs”	3bedbe

summary (x)
#> autotesting package [brolgar, v0.0.6.9000] generated 30 rows of output of the following types:
#>      13 errors
#>      0 warnings
#>      2 messages
#>      15 other diagnosticss
#> That corresponds to 3.75 messages per documented function (which has examples)
#> 
#>         fn_name num_errors num_warnings num_messages num_diagnostics
#> 1     add_n_obs          3           NA           NA               3
#> 2 index_regular         NA           NA           NA               1
#> 3     key_slope          4           NA            2               2
#> 4     keys_near          6           NA           NA               1
#> 5     monotonic         NA           NA           NA               1
#> 6         n_obs         NA           NA           NA               3
#> 7   near_middle         NA           NA           NA               1
#> 8 near_quantile         NA           NA           NA               3
#> 
#>     git hash for package as analysed here:
#>     [698406bbea9b5fd4568eadd96ff5a8cefc8887c2]

^{Created on 2020-10-07 by the reprex package (v0.3.0)}

Further information on autotest output

The output of autotest includes a column yaml_hash. This in turn refers to the yaml specification used to generate the autotests, which can be generated locally by running examples_to_yaml (<path>/<to>/<package>). Those contain the yaml_hash values, and finding the matching value will show you the base code used to trigger the diagnostic messages. The operation column should then provide a sufficient description of what has been mutated with regard to the structure defined in the yaml.

For example, several of the first values are errors for the .data parameter of add_n_obs with the content of the error message being, "Input must inherit from tsibble". The operation in all of the cases describe the mutations which autotest has performed to trigger those error messages. You should be able to suppress these errors by simply specifying in the documentation of that parameter that it is expected to be of class ("tsibble") only.

The text was updated successfully, but these errors were encountered:

njtierney · 2020-09-23T22:27:26Z

Hi @mpadge !

OK great, love these tests, very very cool.

I am on leave from tomorrow so I won't have much opportunity to work on this today, but my focus points from this are:

Ensure all functions contain documentation, and documented examples
Specify the documentation that tsibble is the expected class of add_n_obs() and other similar functions that expect tsibble.
Re-run autotest to check I don't have errors.

Re packgraph, it seems like my actionable from this is to improve documentation, but I was wondering if there were other things to take away from that/improve for next submission?

mpadge · 2020-09-24T07:07:40Z

Thanks for the quick response @njtierney. And yes, the first 2 of those response would be great. It would be especially useful if you could:

ping here when you've done the first, and only the first, then i'll update the autotest output accordingly.
only after that, move on to addressing any of the other points flagged in the autotest output.

You'll be unlikely to get a clean autotest run that returns nothing, but it should ideally at least be free of errors and warnings.

As for packgraph, that is mainly intended to be of aid to potential reviewers, along with the actual graph generated with plot = TRUE, so no particular action expected on your part there.

njtierney · 2020-10-07T00:49:03Z

Hi @mpadge

I have provided examples in the documentation for key_slope and keys_near, is that what was needed for the next step?

mpadge · 2020-10-07T12:17:02Z

Great, thanks @njtierney. I've updated the autotest output in the first comment above. If you could now address as many of those as you deem relevant. A lot of them obviously arise from throwing tabular objects of classes other than tsibble at the functions. This is automatically done because you don't document within the function itself that the .data (or whatever) parameter should be a tsibble. Simply doing that for all functions which appear above should suppress most of those messages, and the rest should hopefully be sufficiently straightforward. Thanks!

njtierney · 2020-10-08T11:41:19Z

Thanks, @mpadge !

I've had a go at resolving these tsibble checks, and a few of the other checks, but I've got a couple of questions about the remaining checks.

For the formula checks, I'm not sure how to intepret these. For example: ":1:1: unexpected ‘<’". Is this suggesting that I check if the input is a formula when passing it into the function
There's a few errors stating, "object ‘height_cm’ not found", I'm not sure why that is happening
I'm not sure about the input, "Function [near_middle] errors on list-columns when submitted as x Error message: comparison of these types is not implemented", is this something to do with ensuring that the types of input for near_middle are the same?
Can you tell me more about lines 29 and 30 - I'm not sure how to interpret these.
- "Function [near_quantile] errors on vector columns with different classes when submitted as probs Error message: no applicable method for ‘round_any’ applied to an object of class “different”"
- "Function [near_quantile] errors on list-columns when submitted as probs Error message: no applicable method for ‘round_any’ applied to an object of class “AsIs” "

mpadge · 2020-10-08T12:00:31Z

That's mostly happening because, yes, the functions currently merely presume the input to be a formula, yet do not actually check for that. The unintelligible error messages are as directly issued by brolgar when a formula is submitted as a character string. You could either strictly restrict such input to formula only (like stopifnot(is(x,"formula"))), or appropriately process results of attempts to coerce. (If you decide to do the latter, it might be useful to know that R itself defines objects as formulas purely by identifying a tilde (~), so you can control a lot by simply grepping for that.) I also think your 2nd point reflects the effect of trying to submit a non-formula value for an expected formula.

Lines 29--30 reflect attempts to convert a vector column to a list (x$x <- I(as.list(x$x))). This will usually fail, yet is always implemented in current autotest checks. Future iterations will allow more flexible user control of what is actually tested. For the moment, feel free to ignore those (unless you actually want to implement conversion from list-columns back to vectors, which can sometimes be a useful thing to do, especially if you envision lots of purrr-type output to be used as input for your package?)

njtierney · 2020-10-13T03:44:00Z

Thanks for that, Mark!

These tests have been great, I've substantially improved the documentation and error/edge case handling as a result.

I've now added tests for formula, first checking for ~ and providing a bit more of a helpful error message than the one R gives, then using is(x, "formula") and providing another error message. Perhaps a bit of overkill, but I'd like to provide more helpful error messages. These testing functions are here, if you are interested.

I'm interested in the conversion of list-columns back into vectors, but I'm not sure what the best practices are here, is it just something like as.vector(list_obj)?

Output of autotest is below (happy to not paste it here if you don't want the issue to get super long),

A few questions about it

do I need to address all of these - are there some issues here that you are particularly concerned about?
I can ask questions about most of these, but there are a lot of rows in the table, and I've been having a guess at solving the issue they refer to - for example, how do I unpack number 38? Is there a guide I can look to to see how these are created and what they expect?
Some messages like, "no applicable method for ‘key_slope’ applied to an object of class “data.frame”", I'm having trouble understanding - there are no specified methods for "data.frame", they just work with tsibble - should I worry about those?

    # install_github("ropenscilabs/autotest", force = TRUE)
    library(autotest)
    packageVersion("autotest")
    #> [1] '0.0.1.281'
    x <- autotest_package(package = "brolgar")
    #> ✓ [1 / 159]: key_slope
    #> ✓ [2 / 159]: key_slope
    #> 
    #> Attaching package: 'dplyr'
    #> The following objects are masked from 'package:stats':
    #> 
    #>     filter, lag
    #> The following objects are masked from 'package:base':
    #> 
    #>     intersect, setdiff, setequal, union
    #> ✓ [3 / 159]: add_n_obs
    #> ✓ [4 / 159]: b_min
    #> ✓ [5 / 159]: b_max
    #> ✓ [6 / 159]: b_median
    #> ✓ [7 / 159]: b_mean
    #> ✓ [8 / 159]: b_range
    #> ✓ [9 / 159]: b_var
    #> ✓ [10 / 159]: b_sd
    #> ✓ [11 / 159]: b_min
    #> ✓ [12 / 159]: b_max
    #> ✓ [13 / 159]: b_median
    #> ✓ [14 / 159]: b_mean
    #> ✓ [15 / 159]: b_range
    #> ✓ [16 / 159]: b_var
    #> ✓ [17 / 159]: b_sd
    #> ✓ [18 / 159]: b_min
    #> ✓ [19 / 159]: b_max
    #> ✓ [20 / 159]: b_median
    #> ✓ [21 / 159]: b_mean
    #> ✓ [22 / 159]: b_range
    #> ✓ [23 / 159]: b_var
    #> ✓ [24 / 159]: b_sd
    #> ✓ [25 / 159]: b_min
    #> ✓ [26 / 159]: b_max
    #> ✓ [27 / 159]: b_median
    #> ✓ [28 / 159]: b_mean
    #> ✓ [29 / 159]: b_range
    #> ✓ [30 / 159]: b_var
    #> ✓ [31 / 159]: b_sd
    #> ✓ [32 / 159]: b_min
    #> ✓ [33 / 159]: b_max
    #> ✓ [34 / 159]: b_median
    #> ✓ [35 / 159]: b_mean
    #> ✓ [36 / 159]: b_range
    #> ✓ [37 / 159]: b_var
    #> ✓ [38 / 159]: b_sd
    #> ✓ [39 / 159]: b_min
    #> ✓ [40 / 159]: b_max
    #> ✓ [41 / 159]: b_median
    #> ✓ [42 / 159]: b_mean
    #> ✓ [43 / 159]: b_range
    #> ✓ [44 / 159]: b_var
    #> ✓ [45 / 159]: b_sd
    #> ✓ [46 / 159]: b_min
    #> ✓ [47 / 159]: b_max
    #> ✓ [48 / 159]: b_median
    #> ✓ [49 / 159]: b_mean
    #> ✓ [50 / 159]: b_range
    #> ✓ [51 / 159]: b_var
    #> ✓ [52 / 159]: b_sd
    #> ✓ [53 / 159]: b_min
    #> ✓ [54 / 159]: b_max
    #> ✓ [55 / 159]: b_median
    #> ✓ [56 / 159]: b_mean
    #> ✓ [57 / 159]: b_range
    #> ✓ [58 / 159]: b_var
    #> ✓ [59 / 159]: b_sd
    #> ✓ [60 / 159]: b_min
    #> ✓ [61 / 159]: b_max
    #> ✓ [62 / 159]: b_median
    #> ✓ [63 / 159]: b_mean
    #> ✓ [64 / 159]: b_range
    #> ✓ [65 / 159]: b_var
    #> ✓ [66 / 159]: b_sd
    #> ✓ [67 / 159]: b_min
    #> ✓ [68 / 159]: b_max
    #> ✓ [69 / 159]: b_median
    #> ✓ [70 / 159]: b_mean
    #> ✓ [71 / 159]: b_range
    #> ✓ [72 / 159]: b_var
    #> ✓ [73 / 159]: b_sd
    #> ✓ [74 / 159]: b_min
    #> ✓ [75 / 159]: b_max
    #> ✓ [76 / 159]: b_median
    #> ✓ [77 / 159]: b_mean
    #> ✓ [78 / 159]: b_range
    #> ✓ [79 / 159]: b_var
    #> ✓ [80 / 159]: b_sd
    #> ✓ [81 / 159]: b_min
    #> ✓ [82 / 159]: b_max
    #> ✓ [83 / 159]: b_median
    #> ✓ [84 / 159]: b_mean
    #> ✓ [85 / 159]: b_range
    #> ✓ [86 / 159]: b_var
    #> ✓ [87 / 159]: b_sd
    #> ✓ [88 / 159]: b_min
    #> ✓ [89 / 159]: b_max
    #> ✓ [90 / 159]: b_median
    #> ✓ [91 / 159]: b_mean
    #> ✓ [92 / 159]: b_range
    #> ✓ [93 / 159]: b_var
    #> ✓ [94 / 159]: b_sd
    #> ✓ [95 / 159]: b_min
    #> ✓ [96 / 159]: b_max
    #> ✓ [97 / 159]: b_median
    #> ✓ [98 / 159]: b_mean
    #> ✓ [99 / 159]: b_range
    #> ✓ [100 / 159]: b_var
    #> ✓ [101 / 159]: b_sd
    #> ✓ [102 / 159]: b_min
    #> ✓ [103 / 159]: b_max
    #> ✓ [104 / 159]: b_median
    #> ✓ [105 / 159]: b_mean
    #> ✓ [106 / 159]: b_range
    #> ✓ [107 / 159]: b_var
    #> ✓ [108 / 159]: b_sd
    #> ✓ [109 / 159]: b_min
    #> ✓ [110 / 159]: b_max
    #> ✓ [111 / 159]: b_median
    #> ✓ [112 / 159]: b_mean
    #> ✓ [113 / 159]: b_range
    #> ✓ [114 / 159]: b_var
    #> ✓ [115 / 159]: b_sd
    #> ✓ [116 / 159]: b_min
    #> ✓ [117 / 159]: b_max
    #> ✓ [118 / 159]: b_median
    #> ✓ [119 / 159]: b_mean
    #> ✓ [120 / 159]: b_range
    #> ✓ [121 / 159]: b_var
    #> ✓ [122 / 159]: b_sd
    #> ✓ [123 / 159]: b_min
    #> ✓ [124 / 159]: b_max
    #> ✓ [125 / 159]: b_median
    #> ✓ [126 / 159]: b_mean
    #> ✓ [127 / 159]: b_range
    #> ✓ [128 / 159]: b_var
    #> ✓ [129 / 159]: b_sd
    #> ✓ [130 / 159]: b_min
    #> ✓ [131 / 159]: b_max
    #> ✓ [132 / 159]: b_median
    #> ✓ [133 / 159]: b_mean
    #> ✓ [134 / 159]: b_range
    #> ✓ [135 / 159]: b_var
    #> ✓ [136 / 159]: b_sd
    #> ✓ [137 / 159]: b_min
    #> ✓ [138 / 159]: b_max
    #> ✓ [139 / 159]: b_median
    #> ✓ [140 / 159]: b_mean
    #> ✓ [141 / 159]: b_range
    #> ✓ [142 / 159]: b_var
    #> ✓ [143 / 159]: b_sd
    #> ✓ [144 / 159]: increasing✓ [144 / 159]: decreasing✓ [144 / 159]: unvarying
    #> ✓ [145 / 159]: increasing✓ [145 / 159]: decreasing✓ [145 / 159]: unvarying
    #> ✓ [146 / 159]: index_regular
    #> ✓ [147 / 159]: key_slope
    #> ✓ [148 / 159]: keys_near
    #> ✓ [149 / 159]: increasing✓ [149 / 159]: decreasing✓ [149 / 159]: unvarying
    #> ✓ [150 / 159]: n_obs
    #> ✓ [151 / 159]: near_middle
    #> ✓ [152 / 159]: near_quantile
    #> ✓ [153 / 159]: nearest_lgl
    #> ✓ [154 / 159]: nearest_lgl
    #> ✓ [155 / 159]: sample_n_keys
    #> ✓ [156 / 159]: sample_frac_keys
    #> ✓ [157 / 159]: sample_n_keys
    #> ✓ [158 / 159]: sample_frac_keys
    #> ✓ [159 / 159]: increasing✓ [159 / 159]: decreasing✓ [159 / 159]: unvarying

    x$yaml_hash <- substring (x$yaml_hash, 1, 6)

    knitr::kable (x, row.names = TRUE)

	type	fn_name	parameter	operation	content	yaml_hash
1	error	key_slope	.data	tabular as data.frame	no applicable method for ‘key_slope’ applied to an object of class “data.frame”	02b934
2	error	key_slope	.data	tabular as tibble	no applicable method for ‘key_slope’ applied to an object of class “c(‘tbl_df’, ‘tbl’, ‘data.frame’)”	02b934
3	error	key_slope	.data	tabular as data.table	no applicable method for ‘key_slope’ applied to an object of class “c(‘data.table’, ‘data.frame’)”	02b934
4	diagnostic	key_slope	.data	tabular structure with new class structure	Function [key_slope] should error when class structure of `data.frame` input is removed.	02b934
5	error	key_slope	formula	(unquoted) formula param as (quoted) character	Input x must be a formula, we see, ‘~height_cmyear’ of class character. Formula should be specified with something on the left hand side of ~ and the right hand side. For more details on formula in R, see `?formula`.	02b934
6	error	add_n_obs	.data	tabular as data.frame	Input must inherit from tsibble	6ecbc3
7	error	add_n_obs	.data	tabular as tibble	Input must inherit from tsibble	6ecbc3
8	error	add_n_obs	.data	tabular as data.table	Input must inherit from tsibble	6ecbc3
9	diagnostic	add_n_obs	.data	tabular structure with new class structure	Function [add_n_obs] should error when class structure of `data.frame` input is removed.	6ecbc3
10	diagnostic	b_min	x	integer vector converted to numeric	Function [b_min] returns different values when assumed int-valued parameter [x] is submitted as double. Error message: different classes when submitted as x	d5a795
11	diagnostic	b_min	x	convert vector input to list-columns	Function [b_min] errors on list-columns when submitted as x Error message: invalid ‘type’ (list) of argument	d5a795
12	diagnostic	b_max	x	integer vector converted to numeric	Function [b_max] returns different values when assumed int-valued parameter [x] is submitted as double. Error message: different classes when submitted as x	b6ca10
13	diagnostic	b_max	x	convert vector input to list-columns	Function [b_max] errors on list-columns when submitted as x Error message: invalid ‘type’ (list) of argument	b6ca10
14	diagnostic	b_median	x	convert vector input to list-columns	Function [b_median] errors on list-columns when submitted as x Error message: comparison of these types is not implemented	ffe0f3
15	diagnostic	b_range	x	integer vector converted to numeric	Function [b_range] returns different values when assumed int-valued parameter [x] is submitted as double. Error message: different classes when submitted as x	ee63e2
16	diagnostic	b_var	x	convert vector input to list-columns	Function [b_var] errors on list-columns when submitted as x Error message: :var(x, na.rm = TRUE, …): is.atomic(x) is not TRUE	597264
17	diagnostic	increasing	x	convert vector input to list-columns	Function [increasing] errors on list-columns when submitted as x Error message: -(length(r) - lag + 1L)]: non-numeric argument to binary operator	cd08e6
18	diagnostic	decreasing	x	convert vector input to list-columns	Function [decreasing] errors on list-columns when submitted as x Error message: -(length(r) - lag + 1L)]: non-numeric argument to binary operator	cd08e6
19	diagnostic	unvarying	x	convert vector input to list-columns	Function [unvarying] errors on list-columns when submitted as x Error message: -(length(r) - lag + 1L)]: non-numeric argument to binary operator	cd08e6
20	diagnostic	index_regular	.data	tabular structure with new class structure	Function [index_regular] should error when class structure of `data.frame` input is removed.	09494b
21	error	keys_near	.data	tabular as data.frame	object ‘height_cm’ not found	1d9240
22	error	keys_near	.data	tabular as tibble	object ‘height_cm’ not found	1d9240
23	error	keys_near	.data	tabular as data.table	object ‘height_cm’ not found	1d9240
24	error	keys_near	.data	rectangular parameter with extended class structure	object ‘height_cm’ not found	1d9240
25	diagnostic	keys_near	.data	tabular structure with new class structure	Function [keys_near] should error when class structure of `data.frame` input is removed.	1d9240
26	error	keys_near	NA	normal function call	object ‘height_cm’ not found	1d9240
27	error	keys_near	NA	error from normal operation	object ‘height_cm’ not found	1d9240
28	diagnostic	n_obs	x	tabular structure with new class structure	Function [n_obs] should error when class structure of `data.frame` input is removed.	138fc0
29	diagnostic	n_obs	names	substitute integer values for logical parameter	Parameter names of function [n_obs] is assumed to be logical, but responds to general integer values.	138fc0
30	diagnostic	near_middle	x	convert vector input to list-columns	Function [near_middle] errors on list-columns when submitted as x Error message: comparison of these types is not implemented	5a6bf1
31	diagnostic	near_quantile	x	convert vector input to list-columns	Function [near_quantile] errors on list-columns when submitted as x Error message: comparison of these types is not implemented	aacdfe
32	diagnostic	near_quantile	probs	custom class definitions for vector input	Function [near_quantile] errors on vector columns with different classes when submitted as probs Error message: no applicable method for ‘round_any’ applied to an object of class “different”	aacdfe
33	diagnostic	near_quantile	probs	convert vector input to list-columns	Function [near_quantile] errors on list-columns when submitted as probs Error message: no applicable method for ‘round_any’ applied to an object of class “AsIs”	aacdfe
34	diagnostic	nearest_lgl	x	custom class definitions for vector input	Function [nearest_lgl] errors on vector columns with different classes when submitted as x Error message: Can’t convert to .	43add7
35	diagnostic	nearest_lgl	x	convert vector input to list-columns	Function [nearest_lgl] errors on list-columns when submitted as x Error message: Can’t convert to .	43add7
36	diagnostic	nearest_lgl	y	custom class definitions for vector input	Function [nearest_lgl] errors on vector columns with different classes when submitted as y Error message: Can’t convert to .	43add7
37	diagnostic	nearest_lgl	y	convert vector input to list-columns	Function [nearest_lgl] errors on list-columns when submitted as y Error message: Can’t convert to .	43add7
38	diagnostic	nearest_lgl	y	ascertain integer range	Parameter [y] permits unrestricted integer inputs	43add7
39	diagnostic	nearest_lgl	x	ascertain integer range	Parameter [x] permits unrestricted integer inputs	43add7
40	error	sample_n_keys	.data	tabular as data.frame	Input must inherit from tsibble	22871f
41	error	sample_n_keys	.data	tabular as tibble	Input must inherit from tsibble	22871f
42	error	sample_n_keys	.data	tabular as data.table	Input must inherit from tsibble	22871f
43	diagnostic	sample_n_keys	.data	tabular structure with new class structure	Function [sample_n_keys] should error when class structure of `data.frame` input is removed.	22871f
44	diagnostic	sample_n_keys	size	ascertain integer range	Parameter [size] responds to integer values in [1, 538]	22871f
45	diagnostic	sample_n_keys	size	match integer range to documentation	Parameter range for size is NOT documented	22871f
46	diagnostic	sample_n_keys	NA	check that description has return value	Function [sample_n_keys] does not specify a return value, yet returns a value of class [tbl_ts, tbl_df, tbl, data.frame]	22871f
47	error	sample_frac_keys	.data	tabular as data.frame	Input must inherit from tsibble	ca0e86
48	error	sample_frac_keys	.data	tabular as tibble	Input must inherit from tsibble	ca0e86
49	error	sample_frac_keys	.data	tabular as data.table	Input must inherit from tsibble	ca0e86
50	diagnostic	sample_frac_keys	.data	tabular structure with new class structure	Function [sample_frac_keys] should error when class structure of `data.frame` input is removed.	ca0e86
51	diagnostic	sample_frac_keys	size	add trivial noise to double parameter	Parameter [size] yields different result when trivial noise is added	ca0e86
52	diagnostic	sample_frac_keys	NA	check that description has return value	Function [sample_frac_keys] does not specify a return value, yet returns a value of class [tbl_ts, tbl_df, tbl, data.frame]	ca0e86

^{Created on 2020-10-13 by the reprex package (v0.3.0)}

mpadge · 2020-10-16T12:22:20Z

Thanks @njtierney for your submission and improvements that have already been made to the package. Don't worry about the remaining autotest output. Most of it can be ignored in this case, and the few things that can't are ultimately going to be optional (like handling list-columns). The testing regime is currently hard-coded, but will soon allow user-specified regimes to be applied. We also plan to implement a system to record written explanations for any categories of tests you choose to disable, with your justifications ultimately incorporated within initial package submission/pre-review templates. None of that is yet possible, so we're happy to just move on, but will likely re-visit with autotest at a later stage, for which we have all the relevant git hash information.

Given that, we would now like to proceed to the formal review stage, for which members of the project's advisory board @stephaniehicks and @lcolladotor have kindly agreed to review your package. They are now requested to perform a two-stage review, the first part involving assessment of the package against the standards as they are currently drafted, with the second being a more "traditional" review. We hope, by the time we proceed to this second component, that many aspects which might otherwise have arisen within a "traditional" unstructured review will already have been covered, and will thereby make the review process notably easier.

Our review system will ultimately perform much of the preceding automated assessment prior to actual submission, and reviewers will be provided with a high-level interactive graphical overview of the package's functions and their inter-relationships. In lieu of the system being that far, reviewers can clone Nick's repo from github.com/njtierney/brolgar, then run the following three lines in the brolgar directory:

remotes::install_github("mpadge/packgraph")
library(packgraph)
pg_graph(".")

That should give you an interactive version something like this:

Instructions for review

@stephaniehicks and @lcolladotor, could you please now asses the brolgar package with respect to the current General Standards for Statistical Software, and the category-specific standards for both Time Series and Exploratory Data Analysis (EDA) software.

Please do this in two phases:

Review the package as it stood at the time of initial assessment (git hash 698406, as given above).
Having done that, checkout package in current status, and note any differences, along with the git hash at that stage.

In each case, please only note those standards which you judge the package not to conform to, along with a description of what you would expect this particular software package to do in order to conform to each standard. When you do that, please provide sufficient information on which standard you are referring to. (The standards themselves are all enumerated, but not yet at a necessarily stable state, so please provide enough information for anyone to clearly know which standard you are referring to regardless of potential changes in nomenclature.) Please also note as a separate list all those standards which you think should not apply to this package, along with brief explanations of why.

Importantly, to aid us in refining the standards which will ultimately guide the peer review of statistical software, we also ask you to please consider whether you perceive any aspects of software (design, functionality, algorithmic implementations or applications, testing, and any other aspects you can think of) which you think might be able to be addressed by standards, and yet which are not addressed by our standards in their current form.

To sum up, please post the following in this issue:

Your 2 itemized lists of standards which the software initially failed to meet, along with an indication of which of that subset of standards the most recent version then actually meets. Please provide git hashes for the repository head in each case.
An indication of any aspects of the software which you think are not addressed by the current standards yet could (potentially) be.

Once you've done that, we'll ask to you proceed to a more general review of the software, for which we'll provide more detail at that time. Thanks all for agreeing to be part of this!

Due date

We would like to have this review phase completed within 4 weeks, so by the 13th of November 2020. We accordingly suggest that you aim to have the first of the two tasks completed within two weeks, by the 30th October.

Could you both please also record approximately how much time you have spent on each review stage. Thank you!

mpadge · 2020-10-20T10:12:20Z

Update for reviewers @stephaniehicks and @lcolladotor, note that this repo now includes an R package which enables you to get a pre-formatted checklist for your reviews by running the following lines:

remotes::install_github("ropenscilabs/statistical-software-review")
library(statsoftrev) # the name of the package
rssr_standards_checklist (category = c ("eda", "time-series"))

That will produce a markdown-formatted checklist in your clipboard ready to paste where you like, or you can use a filename parameter to specify a local file. Thanks @stephaniehicks for the idea there - I trust that it precisely what you had in mind 😄

ping @noamross so you'll be notified of these conversations.

stephaniehicks · 2020-10-28T03:13:18Z

Initial Review of `brolgar` (git hash 698406)

Reviewer: Stephanie Hicks

Approximate time spent reviewing: 4 hours

git clone [email protected]:njtierney/brolgar.git
cd brolgar
git checkout -b ropensci 698406

remotes::install_github("ropenscilabs/statistical-software-review")
library(statsoftrev) # the name of the package
rssr_standards_checklist (category = c ("general", "eda", "time-series"))

General Standards

Applying General Software Standards

Documentation

G1.0 Statistical Software should list at least one primary reference from published academic literature.

Not met as no paper reference in the documentation or package description

Statistical Terminology

G1.1 All statistical terminology should be clarified and unambiguously defined.

As an example, I read through the vignettes/exploratory-modelling.Rmd which assumed the user understands what is mean by "fit a model". Other examples of potential concern: "fit a linear model for each group in a dataset", "fit a linear model for each key in the data", "linear model formula" (not sure if all users would know what is meant by 'formula').

Another example is in the vignettes/longitudinal-data-structures.Rmd. Potential terminology that could be clarified include: "panel data", "cross-sectional data".

Function-level Documentation

G1.2a All internal (non-exported) functions should also be documented in standard roxygen format, along with a final @noRd tag to suppress automatic generation of .Rd files.

e.g. internal functions (non-exported) in R/utils.R are not documented.

Supplementary Documentation

G1.3 Software should include all code necessary to reproduce results which form the basis of performance claims made in associated publications.

Unsure about this as I do not see any associated publications? I tried to knit several of the vignettes and I was able to dot that.

Input Structures

Uni-variate (Vector) Input

G2.0 Provide explicit secondary documentation of any expectations on lengths of inputs (generally implying identifying whether an input is expected to be single- or multi-valued)

I found at least one example in monotonics.R where @param x numeric or integer is not specific about the length of the input.

G2.1 Provide explicit secondary documentation of expectations on data types of all vector inputs (see the above list).

I found at least one example in b_summaries.R where @param x a vector does not specify the data type of the vector input.

G2.2 Appropriately prohibit or restrict submission of multivariate input to parameters expected to be univariate.

I found at least one example in facet-sample.R where I provided a numeric vector of length 2 to the n_facets parameter.

library(ggplot2)
ggplot(heights, aes(x = year, y = height_cm, group = country)) +
  geom_line() +
  facet_sample(n_facets=c(2,5))

This worked, but resulted in the following warning message.

Warning messages:
1: In seq_len(params$n) : first element used of 'length.out' argument
2: In if (nr.plots <= 3L) c(nr.plots, 1L) else if (nr.plots <= 6L) c((nr.plots +  :
  the condition has length > 1 and only the first element will be used
3: In if (nrow * ncol < n) { :
  the condition has length > 1 and only the first element will be used
4: In seq_len(n_strata) : first element used of 'length.out' argument

In the nearest.R function, this also worked and gave no warning / error.

x <- runif(20)
near_middle(x = x,
            middle = runif(10, 0, 1),
            within = 0.2)

G2.3 For univariate character input:
G2.3b Either: use tolower() or equivalent to ensure input of character parameters is not case dependent; or explicitly document that parameters are strictly case-sensitive.

In at least one function, I found missing documentation around case-sensitivity.

index_regular(pisa, Year)

resulted in

Error: `distinct()` must use existing variables.
x `Year` not found in `.data`.
Run `rlang::last_error()` to see where the error occurred.

Missing or Undefined Values

G2.10 Statistical Software should implement appropriate checks for missing data as part of initial pre-processing prior to passing data to analytic algorithms.

In at least one function, I found missing checks for missing data in rows. This gave no message mentioning mentioning the missing data.

heights_na <- heights
heights_na[1,4] <- NA
keys_near(heights_na, height_cm)

G2.11 Where possible, all functions should provide options for users to specify how to handle missing (NA) data, with options minimally including:

Not met. I could not find an options for user-defined NA handling exist.

G2.12 Functions should never assume non-missingness, and should never pass data with potential missing values to any base routines with default na.rm = FALSE-type parameters (such as mean(), sd() or cor()).

See example above.

G2.13 All functions should also provide options to handle undefined values (e.g., NaN, Inf and -Inf), including potentially ignoring or removing such values.

This returns Inf in the values.

heights_na <- heights
heights_na[1,4] <- Inf
keys_near(heights_na, height_cm)

Testing

Test Data Sets

G4.0 Where applicable or practicable, tests should use standard data sets with known properties (for example, the NIST Standard Reference Datasets, or data sets provided by other widely-used R packages).

Not clear if this is OK, but e.g. tests/testthat/test-facet-strata.R uses the heights dataset as provided in this package as opposed to "other widely-used R packges".

Responses to Unexpected Input

G4.2 Appropriate error and warning behaviour of all functions should be explicitly demonstrated through tests. In particular,
G4.2b Explicit tests should demonstrate conditions which trigger every one of those messages, and should compare the result with expected values.

Didn't see this.

G4.3 For functions which are expected to return objects containing no missing (NA) or undefined (NaN, Inf) values, the absence of any such values in return objects should be explicitly tested.

Algorithm Tests

G4.8 Edge condition tests to test that these conditions produce expected behaviour such as clear warnings or errors when confronted with data with extreme properties including but not limited to:
G4.8a Zero-length data

e.g. no warning / error given:

> vec_inc <- c()
> length(vec_inc)
[1] 0
> increasing(vec_inc)
[1] TRUE

G4.8b Data of unsupported types (e.g., character or complex numbers in for functions designed only for numeric data)

No error / warning given for an unsupported data type:

> vec_inc <- c("a", "b", "c")
> increasing(vec_inc)
Error in r[i1] - r[-length(r):-(length(r) - lag + 1L)] : 
  non-numeric argument to binary operator

G4.8c Data with all-NA fields or columns or all identical fields or columns

e.g.

> vec_inc <- c(NA, NA, NA)
> increasing(vec_inc)
[1] NA

G4.9 Noise susceptibility tests Packages should test for expected stochastic behaviour, such as through the following conditions:
G4.9a Adding trivial noise (for example, at the scale of .Machine$double.eps) to data does not meaningfully change results

Didn't see this

G4.9b Running under different random seeds or initial conditions does not meaningfully change results

Didn't see this, but also don't expect it to change much.

EDA Standards

Documentation Standards

EA1.0 Identify one or more target audiences for whom the software is intended

Didn't see this explicitly mentioned.

EA1.2 Identify the kinds of questions the software is intended to help explore; for example, are these questions: - inferential? - predictive? - associative? - causal? - (or other modes of statistical enquiry?)

The plotting functions seem pretty exploratory / associative, but not explicitly mentioned. Though the plotting functions are also useful for exploring modeling results (i.e. more inferential).

Visualization and Summary Output

EA5.0 Graphical presentation in EDA software should be as accessible as possible or practicable. In particular, EDA software should consider accessibility in terms of:
EA5.0b Default colour schemes should be carefully constructed to ensure accessibility.*

Default ggplot2, but this can be altered by the user.

General Standards for Visualization (Static and Dynamic)

EA5.4 All visualisations should include units on all axes, with sensibly rounded values (for example, as produced by the pretty() function).

Again, just relying on ggplot2 here.

Testing

Graphical Output

EA6.1 The properties of graphical output from EDA software should be explicitly tested, for example via the vdiffr package or equivalent.

Didn't see this?

Time Series Standards

As this package focuses on longitudinal data (e.g. panel data, cross-sectional data, and time series), I struggled to apply the Time Series Standards only because the functions are meant to be broader than just time series. So I decided to remove it as one of the standards. Happy to discuss and loop back to this though.

stephaniehicks · 2020-10-28T03:14:44Z

Re the second item requested from @mpadge: "An indication of any aspects of the software which you think are not addressed by the current standards yet could (potentially) be."

Honestly, my biggest struggle was applying the time series standards, when this package is designed to be broader than just time series. So I wasn't sure how to handle that.

stephaniehicks · 2020-10-28T03:17:21Z

@mpadge I'm not sure I understand the second phase of review. Do I proceed now with a "traditional review" or do I wait? Also, could you provide more guidance on what is meant by that? Thanks!

mpadge · 2020-10-28T09:28:06Z

Thank you very much @stephaniehicks! The next phase is now for @njtierney to address the issues you've addressed here, ping on this issue when he has done so, and for you to then re-assess the above points in response to his changes. Only after that do you need to commence a "traditional review". So over to you @njtierney ... 😄

njtierney · 2020-10-29T23:13:02Z

Wonderful, thank you @stephaniehicks for the thougtful initial review, and @mpadge for organising - I'll get to this next week, looking forward to it.

Cheers!

lcolladotor · 2020-12-01T15:57:24Z

Hi @njtierney,

Sorry for the delay! I really liked your package and I believe that it is of quite useful. Going through the rOpenSci standards, I think that there's a bit of room for improvement, mostly in relation to handling of missing data and a bit of editing in the vignettes to explain more concepts to make it easier for new users to use and understand brolgar. For example, http://brolgar.njtierney.com/articles/mixed-effects-models.html could explain a bit more whey you would want to use mixed effects models and what to look for in the plots. Though I'm quite pleased to see multiple vignettes that showcase all the different pieces of brolgar and how it can be used with other packages (from the data structures with tsibble to the mixed effects with modelr).

I think that you might want to use warning = FALSE and/or message = FALSE in some code chunks to hide some messages that are currently included in the README/vignettes that might confuse new users. For example see http://brolgar.njtierney.com/index.html#linking-individuals-back-to-the-data (which seems like a warning that might need to be resolved), http://brolgar.njtierney.com/index.html#further-reading (which seems like it's not affecting the plots) and http://brolgar.njtierney.com/articles/finding-features.html#accessing-sets-of-features (which seems like warnings you might want to address).

Let me know if I should clarify any of my comments to the rOpenSci standards below.

Best,
Leo

Review of git hash `698406`

Time spent: about 2.5-3 hrs on the package and 2-3 hours reading the standards.

git clone [email protected]:njtierney/brolgar.git
cd brolgar
git checkout -b ropensci 698406

remotes::install_local(dependencies = TRUE)

General Standards

Documentation

G1.0 Statistical Software should list at least one primary reference from published academic literature.

README refers to papers for tsibble and other similar packages, but not one for brolgar. Note that it's Bruce Swihart, not Bryan.

Statistical Terminology

G1.1 All statistical terminology should be clarified and unambiguously defined.

The README section on Using brolgar: We need to talk about data is very well written and clarifies important keywords. I also liked the progression from the README to the Getting Started vignette, to more specialized ones. However, some more work could be done here to help new users in the field. For example, the Getting Started vignette Finding features in longitudinal data section says For example, the five number summary https://github.com/njtierney/brolgar/blob/698406bbea9b5fd4568eadd96ff5a8cefc8887c2/vignettes/getting-started.Rmd#L276 without clarifying what those five numbers are (minimum, 25th quantile, median or 50th quantile, 75th quantile, maximum). Linking to https://en.wikipedia.org/wiki/Quartile might be useful.

Function-level Documentation

G1.2 Software should use roxygen2 to document all functions.
G1.2a All internal (non-exported) functions should also be documented in standard roxygen2 format, along with a final @noRd tag to suppress automatic generation of .Rd files.
Missing a few like FacetStrata() at https://github.com/njtierney/brolgar/blob/698406bbea9b5fd4568eadd96ff5a8cefc8887c2/R/facet-strata.R#L77 and those at https://github.com/njtierney/brolgar/blob/698406bbea9b5fd4568eadd96ff5a8cefc8887c2/R/utils.R.

Supplementary Documentation

G1.3 Software should include all code necessary to reproduce results which form the basis of performance claims made in associated publications.
Not applicable
G1.4 Software should include code necessary to compare performance claims with alternative implementations in other R packages.
No claims have been made. Alternative implementations are cited.

Input Structures

Uni-variate (Vector) Input

Tabular Input

G2.6 Software should accept as input as many of the above standard tabular forms as possible, including extension to domain-specific forms.
I believe this is satisfied using the tsibble package and data.frame/vector inputs. Like, other classes might exist in other packages, but I would say that this is plenty enough to satisfy this standard.
G2.7 Software should provide appropriate conversion or dispatch routines as part of initial pre-processing to ensure that all other sub-functions of a package receive inputs of a single defined class or type.
This is done using the tsibble package.
G2.8 Software should issue diagnostic messages for type conversion in which information is lost (such as conversion of variables from factor to character; standardisation of variable names; or removal of meta-data such as those associated with sf-format data) or added (such as insertion of variable or column names where none were provided).
I don't see such messages, but I don't see any coercion either. So I'll say that this is satisfied.
G2.9 Software should ensure that extraction or filtering of single columns from tabular inputs should not presume any particular default behaviour, and should ensure all column-extraction operations behave consistently regardless of the class of tabular data used as input.
The use of the id column means to me that no default order is assumed.

Missing or Undefined Values

G2.10 Statistical Software should implement appropriate checks for missing data as part of initial pre-processing prior to passing data to analytic algorithms.
This doesn't seem to have been considered. Like near_quantile(c(NA, 1), 0.5) returns an error due to na.rm = FALSE and there's no way right now to specify na.rm = TRUE for the functions used in near_quantile(). For the b_summaries functions, this is implemented at https://github.com/njtierney/brolgar/blob/698406bbea9b5fd4568eadd96ff5a8cefc8887c2/R/b_summaries.R.
G2.11 Where possible, all functions should provide options for users to specify how to handle missing (NA) data, with options minimally including:
Not currently considered as described in the previous bullet point.
G2.11a error on missing data
G2.11b ignore missing data with default warnings or messages issued
G2.11c replace missing data with appropriately imputed values
G2.12 Functions should never assume non-missingness, and should never pass data with potential missing values to any base routines with default na.rm = FALSE-type parameters (such as mean(), sd() or cor()).
As shown with near_quantile() the user can't specify na.rm = TRUE right now.
G2.13 All functions should also provide options to handle undefined values (e.g., NaN, Inf and -Inf), including potentially ignoring or removing such values.
Not currently done.

Output Structures

G3.0 Statistical Software which enables outputs to be written to local files should parse parameters specifying file names to ensure appropriate file suffices are automatically generated where not provided.
Not applicable.

Testing

Test Data Sets

G4.0 Where applicable or practicable, tests should use standard data sets with known properties (for example, the NIST Standard Reference Datasets, or data sets provided by other widely-used R packages).
I'm not familiar with example longitudinal data so I can't comment much here, however, I'll leave it unchecked in case the developers want to add some examples with standard data. On a second thought, the gapminder example at http://brolgar.njtierney.com/articles/longitudinal-data-structures.html satisfies this standard.
G4.1 Data sets created within, and used to test, a package should be exported (or otherwise made generally available) so that users can confirm tests and run examples.

Responses to Unexpected Input

G4.2 Appropriate error and warning behaviour of all functions should be explicitly demonstrated through tests. In particular,
G4.2a Every message produced within R code by stop(), warning(), message(), or equivalent should be unique
Not currently implemented since there are no warnings() in the code.
G4.2b Explicit tests should demonstrate conditions which trigger every one of those messages, and should compare the result with expected values.
Not currently implemented, but is suggested by earlier standards on missing data and data types.
G4.3 For functions which are expected to return objects containing no missing (NA) or undefined (NaN, Inf) values, the absence of any such values in return objects should be explicitly tested.
Not currently implemented.

Algorithm Tests

Extended tests

G4.10 Extended tests should included and run under a common framework with other tests but be switched on by flags such as as a <MYPKG>_EXTENDED_TESTS=1 environment variable.
No extended tests right now.
G4.11 Where extended tests require large data sets or other assets, these should be provided for downloading and fetched as part of the testing workflow.
Not applicable.
G4.11a When any downloads of additional data necessary for extended tests fail, the tests themselves should not fail, rather be skipped and implicitly succeed with an appropriate diagnostic message.
Not applicable.
G4.12 Any conditions necessary to run extended tests such as platform requirements, memory, expected runtime, and artefacts produced that may need manual inspection, should be described in developer documentation such as a CONTRIBUTING.md or tests/README.md file.
Not applicable.

EDA Standards

Documentation Standards

EA1.0 Identify one or more target audiences for whom the software is intended
EA1.1 Identify the kinds of data the software is capable of analysing (see Kinds of Data below).
I feel that this is satisfied in the README.
EA1.2 Identify the kinds of questions the software is intended to help explore; for example, are these questions: - inferential? - predictive? - associative? - causal? - (or other modes of statistical enquiry?)
This is currently not clear. I think that it's mostly for associative and to avoid overplotting.
EA1.3 Identify the kinds of data each function is intended to accept as input

Input Data

Index Columns

EA2.0 EDA Software which accepts standard tabular data and implements or relies upon extensive table filter and join operations should utilise an index column system
EA2.1 All values in an index column must be unique, and this uniqueness should be affirmed as a pre-processing step for all input data.
EA2.2 Index columns should be explicitly identified, either:
EA2.2a by using an appropriate class system, or
EA2.2b through setting an attribute on a table, x, of attr(x, "index") <- <index_col_name>.
Satisfied by EA2.2a.
EA2.3 Table join operations should not be based on any assumed variable or column names
Hm... well, it's based on the index column by left_join(by = "id"), though this is done using dplyr. I think that the current implementation satisfies the standard, though in a way it isn't since the user has to specify the by argument instead of relying on some structure of the object. However, the current results make it easy to work with other packages such as ggplot2.

Multi-tabular input

EA2.4 Use and demand an explicit class system for such input (for example, via the DM package).
Not applicable. The classes are all implemented in tsibble.
EA2.5 Ensure all individual tables follow the above standards for Index Columns

Classes and Sub-Classes

EA2.6 Routines appropriately process vector input of custom classes, including those which do not inherit from the vector class
EA2.7 Routines should appropriately process vector data regardless of additional attributes
EA2.8 EDA routines appropriately process tabular input of custom classes, ideally by means of a single pre-processing routine which converts tabular input to some standard form subsequently passed to all analytic routines.
EA2.9 EDA routines accept and appropriately process tabular input in which individual columns may be of custom sub-classes including additional attributes.

Given the use of tsibble and how it nicely integrates with dplyr and ggplot2, brolgar doesn't need to do much besides show how to use all three packages in the examples. facet_sample() and facet_strata() extend ggplot2 to use data from tsibble.

Analytic Algorithms

Return Results / Output Data

EA4.0 EDA Software should ensure all return results have types which are consistent with input types. For example, sum, min, or max values applied to integer-type vectors should return integer values, while mean or var will generally return numeric types.
Tested with class(b_min(c(1L, 2L))).
EA4.1 EDA Software should implement parameters to enable explicit control of numeric precision
Not implemented on feat_five_num() and related functions. I'm not sure if all the functions used by b_summaries https://github.com/njtierney/brolgar/blob/698406bbea9b5fd4568eadd96ff5a8cefc8887c2/R/b_summaries.R have a tol argument or not. In any way, the user would have to inspect what each b_summary function is using internally and then check if that function, say stats::IQR() used by b_iqr(), has arguments for controling the numeric precision.
EA4.2 The primary routines of EDA Software should return objects for which default print and plot methods give sensible results. Default summary methods may also be implemented.
Not applicable.

Visualization and Summary Output

EA5.0 Graphical presentation in EDA software should be as accessible as possible or practicable. In particular, EDA software should consider accessibility in terms of:
EA5.0a *Typeface sizes should default to sizes which explicitly enhance accessibility
Visualizations are based on ggplot2. Maybe a specific theme could be used in brolgar's examples to increase the font size and utilize accesible colors (EA5.0b below).
EA5.0b Default colour schemes should be carefully constructed to ensure accessibility.*
See EA5.0a.
EA5.1 Any explicit specifications of typefaces which override default values should consider accessibility
Not applicable as this is done using ggplot2.

Summary and Screen-based Output

EA5.2 Screen-based output should never rely on default print formatting of numeric types, rather should also use some version of round(., digits), formatC, sprintf, or similar functions for numeric formatting according the parameter described in EDA4.2.
Not applicable.
EA5.3 Column-based summary statistics should always indicate the storage.mode, class, or equivalent defining attribute of each column (as, for example, implemented in the default print.tibble method).
Hm... I'm not sure if this could be implemented for the b_summaries. I'm not sure how this information would be useful though, or when it would be used by brolgar.

General Standards for Visualization (Static and Dynamic)

EA5.4 All visualisations should include units on all axes, with sensibly rounded values (for example, as produced by the pretty() function).
Currently units are not shown in the axes on the example plots, such as those in the README.

Dynamic Visualization

EA5.5 Any packages which internally bundle libraries used for dynamic visualization and which are also bundled in other, pre-existing R packages, should explain the necessity and advantage of re-bundling that library.
Not applicable.

Testing

Return Values

EA6.0 Return values from all functions should be tested, including tests for the following characteristics:
EA6.0a Classes and types of objects
EA6.0b Dimensions of tabular objects
EA6.0c Column names (or equivalent) of tabular objects
EA6.0d *Classes or types of all columns contained within data.frame-type tabular objects *
EA6.0e Values of single-valued objects; for numeric values either using testthat::expect_equal() or equivalent with a defined value for the tolerance parameter, or using round(..., digits = x) with some defined value of x prior to testing equality.

These standards feel redundant with the general ones.

Graphical Output

EA6.1 The properties of graphical output from EDA software should be explicitly tested, for example via the vdiffr package or equivalent.

Time Series Standards

Input data structures and validation

Not applicable as this is handled by tsibble.

TS1.0 Time Series Software should use and rely on explicit class systems developed for representing time series data, and should not permit generic, non-time-series input
TS1.1 Time Series Software should explicitly document the types and classes of input data able to be passed to each function.
TS1.2 Time Series Software should accept input data in as many time series specific classes as possible.
TS1.3 Time Series Software should implement validation routines to confirm that inputs are of acceptable classes (or represented in otherwise appropriate ways for software which does not use class systems).
TS1.4 Time Series Software should implement a single pre-processing routine to validate input data, and to appropriately transform it to a single uniform type to be passed to all subsequent data-processing functions (the tsbox package provides one convenient approach for this).
TS1.5 The pre-processing function described above should maintain all time- or date-based components or attributes of input data.
TS1.6 The software should ensure strict ordering of the time, frequency, or equivalent ordering index variable.
TS1.7 Any violations of ordering should be caught in the pre-processing stages of all functions.

Time Intervals and Relative Time

Not applicable.

TS1.8 Accept inputs defined via the units package for attributing SI units to R vectors.
TS1.9 Where time intervals or periods may be days or months, be explicit about the system used to represent such, particularly regarding whether a calendar system is used, or whether a year is presumed to have 365 days, 365.2422 days, or some other value.

Pre-processing and Variable Transformation

Missing Data

TS2.0 Appropriate checks for missing data, and associated transformation routines, should be performed as part of initial pre-processing prior to passing data to analytic algorithms.
Not implemented yet as discussed in the general standards above.
TS2.1 Time Series Software which presumes or requires regular data should only allow explicit missing values, and should issue appropriate diagnostic messages, potentially including errors, in response to any implicit missing values.
tsibble::as_tsibble(regular) has a regular = logical(1) parameter that controls this.
TS2.2 Where possible, all functions should provide options for users to specify how to handle missing data, with options minimally including:
TS2.2a error on missing data.
This could potentially be done in the features() function and not in the b_summaries.
TS2.2b warn or ignore missing data, and proceed to analyse irregular data, ensuring that results from function calls with regular yet missing data return identical values to submitting equivalent irregular data with no missing values.
Not currently done.
TS2.2c replace missing data with appropriately imputed values.
Likely not applicable since brolgar is meant to help visualize the current data. I'm guessing that such imputation would be done in tsibble by the user.
TS2.3 Functions should never assume non-missingness, and should never pass data with potential missing values to any base routines with default na.rm = FALSE-type parameters (such as mean(), sd() or var()).
Redundant with a general standard.

Stationarity

Not applicable.

TS2.4 Consider stationarity of all relevant moments - typically first (mean) and second (variance) order, or otherwise document why such consideration may be restricted to lower orders only.
TS2.5 Explicitly document all assumptions and/or requirements of stationarity
TS2.6 Implement appropriate checks for all relevant forms of stationarity, and either:
TS2.6a issue diagnostic messages or warnings; or
TS2.6b enable or advise on appropriate transformations to ensure stationarity.

Covariance Matrices

Not applicable.

TS2.7 Incorporate a system to ensure that both row and column orders follow the same ordering as the underlying time series data. This may, for example, be done by including the index attribute of the time series data as an attribute of the covariance matrix.
TS2.8 Where applicable, covariance matrices should also include specification of appropriate units.

Analytic Algorithms

Forecasting

Not applicable.

TS3.0 Provide tests to demonstrate at least one case in which errors widen appropriately with forecast horizon.
TS3.1 If possible, provide at least one test which violates TS3.0
TS3.2 Document the general drivers of forecast errors or horizons, as demonstrated via the particular cases of TS3.0 and TS3.1
TS3.3 Either:
TS3.3a Document, preferable via an example, how to trim forecast values based on a specified error margin or equivalent; or
TS3.3b Provide an explicit mechanism to trim forecast values to a specified error margin, either via an explicit post-processing function, or via an input parameter to a primary analytic function.

Return Results

Not applicable.

TS4.0 Return values should either:
TS4.0a Be in same class as input data, for example by using the tsbox package to re-convert from standard internal format (see 1.4, above); or
TS4.0b Be in a unique, preferably class-defined, format.
TS4.1 Any units included as attributes of input data should also be included within return values.
TS4.2 The type and class of all return values should be explicitly documented.
TS4.3 Return values should explicitly include all appropriate units and/or time scales

Data Transformation

Not applicable.

TS4.4 Document the effect of any such transformations on forecast data, including potential effects on both first- and second-order estimates.
TS4.5 In decreasing order of preference, either:
TS4.5a Provide explicit routines or options to back-transform data commensurate with original, non-stationary input data
TS4.5b Demonstrate how data may be back-transformed to a form commensurate with original, non-stationary input data.
TS4.5c Document associated limitations on forecast values

Forecasting

Not applicable.

TS4.6 Time Series Software which implements or otherwise enables forecasting should return either:
TS4.6a A distribution object, for example via one of the many packages described in the CRAN Task View on Probability Distributions (or the new distributional package as used in the fable package for time-series forecasting).
TS4.6b For each variable to be forecast, predicted values equivalent to first- and second-order moments (for example, mean and standard error values).
TS4.6c Some more general indication of error involved with forecast estimates.
TS4.7 Ensure that forecast (modelled) values are clearly distinguished from observed (model or input) values, either (in this case in no order of preference) by
TS4.7a Returning forecast values alone
TS4.7b Returning distinct list items for model and forecast values
TS4.7c Combining model and forecast values into a single return object with an appropriate additional column clearly distinguishing the two kinds of data.

Visualization

TS5.0 Implement default plot methods for any implemented class system.
Not applicable.
TS5.1 When representing results in temporal domain(s), ensure that one axis is clearly labelled "time" (or equivalent), with continuous units.
Up to the user when using ggplot2.
TS5.2 Default to placing the "time" (or equivalent) variable on the horizontal axis.
That's the case in the examples, but it's up to the user.
TS5.3 Ensure that units of the time, frequency, or index variable are printed by default on the axis.
Up to the user when using ggplot2.
TS5.4 For frequency visualization, abscissa spanning $[-\pi, \pi]$ should be avoided in favour positive units of $[0, 2\pi]$ or $[0, 0.5]$, in all cases with appropriate additional explanation of units.
Not applicable.
TS5.5 Provide options to determine whether plots of data with missing values should generate continuous or broken lines.
Not implemented yet and it depends on ggplot2's behavior for missing data.
TS5.6 By default indicate distributional limits of forecast on plot
Not applicable.
TS5.7 By default include model (input) values in plot, as well as forecast (output) values
Not applicable.
TS5.8 By default provide clear visual distinction between model (input) values and forecast (output) values.
Not applicable.

R session info

> library(brolgar)
> library(sessioninfo)
> options(width = 120)
> session_info()
─ Session info ───────────────────────────────────────────────────────────────────────────────────────────────────────
 setting  value
 version  R version 4.0.3 (2020-10-10)
 os       macOS Big Sur 10.16
 system   x86_64, darwin17.0
 ui       X11
 language (EN)
 collate  en_US.UTF-8
 ctype    en_US.UTF-8
 tz       America/New_York
 date     2020-12-01

─ Packages ───────────────────────────────────────────────────────────────────────────────────────────────────────────
 package        * version    date       lib source
 anytime          0.3.9      2020-08-27 [1] CRAN (R 4.0.2)
 assertthat       0.2.1      2019-03-21 [1] CRAN (R 4.0.2)
 brolgar        * 0.0.6.9000 2020-12-01 [1] local
 cli              2.2.0      2020-11-20 [1] CRAN (R 4.0.2)
 colorspace       2.0-0      2020-11-11 [1] CRAN (R 4.0.2)
 crayon           1.3.4      2017-09-16 [1] CRAN (R 4.0.2)
 distributional   0.2.1      2020-10-06 [1] CRAN (R 4.0.2)
 dplyr            1.0.2      2020-08-18 [1] CRAN (R 4.0.2)
 ellipsis         0.3.1      2020-05-15 [1] CRAN (R 4.0.2)
 fabletools       0.2.1      2020-09-03 [1] CRAN (R 4.0.2)
 fansi            0.4.1      2020-01-08 [1] CRAN (R 4.0.2)
 farver           2.0.3      2020-01-16 [1] CRAN (R 4.0.2)
 generics         0.1.0      2020-10-31 [1] CRAN (R 4.0.2)
 ggplot2          3.3.2      2020-06-19 [1] CRAN (R 4.0.2)
 glue             1.4.2      2020-08-27 [1] CRAN (R 4.0.2)
 gtable           0.3.0      2019-03-25 [1] CRAN (R 4.0.2)
 lifecycle        0.2.0      2020-03-06 [1] CRAN (R 4.0.2)
 lubridate        1.7.9.2    2020-11-13 [1] CRAN (R 4.0.2)
 magrittr         2.0.1      2020-11-17 [1] CRAN (R 4.0.2)
 munsell          0.5.0      2018-06-12 [1] CRAN (R 4.0.2)
 pillar           1.4.7      2020-11-20 [1] CRAN (R 4.0.2)
 pkgconfig        2.0.3      2019-09-22 [1] CRAN (R 4.0.2)
 purrr            0.3.4      2020-04-17 [1] CRAN (R 4.0.2)
 R6               2.5.0      2020-10-28 [1] CRAN (R 4.0.2)
 Rcpp             1.0.5      2020-07-06 [1] CRAN (R 4.0.2)
 rlang            0.4.9      2020-11-26 [1] CRAN (R 4.0.2)
 scales           1.1.1      2020-05-11 [1] CRAN (R 4.0.2)
 sessioninfo    * 1.1.1      2018-11-05 [1] CRAN (R 4.0.2)
 tibble           3.0.4      2020-10-12 [1] CRAN (R 4.0.2)
 tidyr            1.1.2      2020-08-27 [1] CRAN (R 4.0.2)
 tidyselect       1.1.0      2020-05-11 [1] CRAN (R 4.0.2)
 tsibble          0.9.3      2020-10-06 [1] CRAN (R 4.0.2)
 vctrs            0.3.5      2020-11-17 [1] CRAN (R 4.0.2)
 withr            2.3.0      2020-09-22 [1] CRAN (R 4.0.2)

[1] /Library/Frameworks/R.framework/Versions/4.0/Resources/library

njtierney mentioned this issue Oct 8, 2020

Error in eval(substitute(fn)): object 'i' not found ropensci-review-tools/autotest#22

Closed

This was referenced Oct 28, 2020

Revisions of general standards mpadge/hackmd-stat-software#1

Open

Revisons of time series standards mpadge/hackmd-stat-software#2

Open

sckott added 6/approved-silver-v0.0.1 and removed 6/approved-silver-v0.0.1 labels Jul 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

brolgar submission #1

brolgar submission #1

mpadge commented Sep 23, 2020 •

edited

Loading

njtierney commented Sep 23, 2020

mpadge commented Sep 24, 2020

njtierney commented Oct 7, 2020

mpadge commented Oct 7, 2020

njtierney commented Oct 8, 2020

mpadge commented Oct 8, 2020

njtierney commented Oct 13, 2020

mpadge commented Oct 16, 2020

mpadge commented Oct 20, 2020

stephaniehicks commented Oct 28, 2020 •

edited

Loading

stephaniehicks commented Oct 28, 2020

stephaniehicks commented Oct 28, 2020

mpadge commented Oct 28, 2020

njtierney commented Oct 29, 2020

lcolladotor commented Dec 1, 2020 •

edited

Loading

brolgar submission #1

brolgar submission #1

Comments

mpadge commented Sep 23, 2020 • edited Loading

Further information on autotest output

njtierney commented Sep 23, 2020

mpadge commented Sep 24, 2020

njtierney commented Oct 7, 2020

mpadge commented Oct 7, 2020

njtierney commented Oct 8, 2020

mpadge commented Oct 8, 2020

njtierney commented Oct 13, 2020

mpadge commented Oct 16, 2020

Instructions for review

Due date

mpadge commented Oct 20, 2020

stephaniehicks commented Oct 28, 2020 • edited Loading

Initial Review of brolgar (git hash 698406)

Documentation

Statistical Terminology

Function-level Documentation

Supplementary Documentation

Input Structures

Uni-variate (Vector) Input

Missing or Undefined Values

Testing

Test Data Sets

Responses to Unexpected Input

Algorithm Tests

Documentation Standards

Visualization and Summary Output

General Standards for Visualization (Static and Dynamic)

Testing

Graphical Output

stephaniehicks commented Oct 28, 2020

stephaniehicks commented Oct 28, 2020

mpadge commented Oct 28, 2020

njtierney commented Oct 29, 2020

lcolladotor commented Dec 1, 2020 • edited Loading

Review of git hash 698406

Documentation

Statistical Terminology

Function-level Documentation

Supplementary Documentation

Input Structures

Uni-variate (Vector) Input

Tabular Input

Missing or Undefined Values

Output Structures

Testing

Test Data Sets

Responses to Unexpected Input

Algorithm Tests

Extended tests

Documentation Standards

Input Data

Index Columns

Multi-tabular input

Classes and Sub-Classes

Analytic Algorithms

Return Results / Output Data

Visualization and Summary Output

Summary and Screen-based Output

General Standards for Visualization (Static and Dynamic)

Dynamic Visualization

Testing

Return Values

Graphical Output

Input data structures and validation

Time Intervals and Relative Time

Pre-processing and Variable Transformation

Missing Data

Stationarity

Covariance Matrices

Analytic Algorithms

Forecasting

Return Results

Data Transformation

Forecasting

Visualization

mpadge commented Sep 23, 2020 •

edited

Loading

stephaniehicks commented Oct 28, 2020 •

edited

Loading

Initial Review of `brolgar` (git hash 698406)

lcolladotor commented Dec 1, 2020 •

edited

Loading

Review of git hash `698406`