Ignore NAs when printing an archive's min/max time_value

[nmdefries]

Description

Toggle on the na.rm option (via the custom Min and Max functions) so even if an epi_archive's time_value field contains some missing values (NA) printed min/max values are still useful. Prints NA if all time_values are missing and "no non-missing arguments" warning is raised, or if an un-caught warning occurs.

Changes

• R/archive.R - added na.rm setting and try-catch to handle all-missing case.
• man/as_epi_df.Rd - not my changes, showed up when I re-document()ed

Fixes

Closes #106

[brookslogan]

Thanks, I like the described behavior. Implementation-wise, I'm not sure if the warning message could vary between different versions of R [so the current check might be brittle]. (+ pattern-wise we might want to keep a habit of using Warn with a parent arg rather than warning(w) to rethrow... not sure it's better here but it would be in more complicated cases where we don't want to lose stack traces that we could enable.) Maybe the tryCatch could be replaced with a check on whether the vector is length-0 or all NA (or NaN)?

[brookslogan]

Not sure if I was too afraid of "brittleness" above. Slight tweak to the current logic to get it closer to how the warning is triggered:

|| all(is.na(self$DT$time_value)) || all(is.nan(self$DT$time_value)) looks like it wouldn't catch a mix of NAs and NaNs, so it'd seem like we'd want || all(is.na(self$DT$time_value) | is.nan(self$DT$time_value)) to detect a mix of NA and NaNs. However, it turns out that using is.nan isn't even necessary: ?is.na states that:

The default method for 'is.na' applied to an atomic vector returns a logical vector of the same length as its argument 'x', containing 'TRUE' for those elements marked 'NA' or, for numeric or complex vectors, 'NaN', and 'FALSE' otherwise. (A complex value is regarded as 'NA' if either its real or imaginary part is 'NA' or 'NaN'.) 'dim', 'dimnames' and 'names' attributes are copied to the result.
So we can just change the above to || all(is.na(self$DT$time_value)). I did that in the commit above.

Side note from testing: we can't directly construct an epi_archive with all-NaN time_values, although I see no use cases for this.

as_epi_archive(tibble::tibble(geo_value=1,time_value=NaN,version=1,value=1))
#> Error in if (is.numeric(time_value) && all(time_value == as.integer(time_value)) &&  (from utils.R#93) : 
#>  missing value where TRUE/FALSE needed

I do see a use case for all-NA time values in an epi_archive, to provide version tracking for non-time-series data (or maybe time series in a nested format for some reason?) using epi_archive, which insists on a time_value column.

[brookslogan]

To clarify in case we revisit this thread: we are currently able to directly construct an epi_archive with all NA time_values, just not all NaN ones. This PR properly prints the time_value range of an epi_archive given any mixture of these that we can directly construct or sneak in via mutating the DT contents or reseating the DT field.

[nmdefries]

Thanks for the fix.

Side note from testing: we can't directly construct an epi_archive with all-NaN time_values

In my testing, it seems that we can. The problem with construction is that the time_type auto-detection doesn't work -- I'm getting a

Error in if (is.numeric(time_value) && all(time_value == as.integer(time_value)) &&  : 
  missing value where TRUE/FALSE needed

but if we set time_type manually, the archive is successfully created.