averissimo commented 2 months ago

Pull request

Closes #299

Changes description

Prevent missing from being passed in call tree until it fails on open_font_dev
Adds test case

github-actions[bot] commented 2 months ago

Unit Tests Summary

1 files 9 suites 15s :stopwatch: 54 tests 54 :white_check_mark: 0 :zzz: 0 :x: 364 runs 364 :white_check_mark: 0 :zzz: 0 :x:

Results for commit 4d6ca86c.

:recycle: This comment has been updated with latest results.

github-actions[bot] commented 2 months ago

Unit Test Performance Difference

Test Suite	$Status$	Time on `main`	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
generics	👶		$+0.03$	$+3$	$0$	$0$	$0$

Additional test case details

| Test Suite | $Status$ | Time on `main` | $±Time$ | Test Case | |:-----|:----:|:----:|:----:|:-----| | generics | 👶 | | $+0.00$ | nlines_list_argument_is_broken_down_in_3_lines_with_max_width_defined | | generics | 👶 | | $+0.00$ | nlines_small_string_produces_1_line_as_output | | generics | 👶 | | $+0.02$ | nlines_string_is_broken_down_in_2_lines_with_max_width_defined |

Results for commit 9d2dd033b67e78f0166be8215ebf771c0a1f6e73

♻️ This comment has been updated with latest results.

github-actions[bot] commented 2 months ago

Code Coverage Summary

Filename             Stmts    Miss  Cover    Missing
-----------------  -------  ------  -------  -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
R/format_value.R       195      12  93.85%   88, 105-112, 187, 296, 396, 407, 415
R/generics.R           126      12  90.48%   142, 281-285, 487, 499, 530, 560, 668, 681, 702-709
R/labels.R              55       7  87.27%   51, 57, 66, 107, 133, 142, 146
R/matrix_form.R        683      40  94.14%   121, 492-493, 585, 595-598, 616, 647, 737-738, 752-757, 787-790, 850-851, 939-940, 967, 995, 1047, 1209, 1245, 1248-1252, 1302, 1350, 1353, 1357
R/mpf_exporters.R      277      20  92.78%   2, 100-102, 147, 183, 227, 230, 235, 414-420, 424, 427, 431, 479, 557
R/page_size.R           45       2  95.56%   10, 221
R/pagination.R         759      52  93.15%   327-330, 435-450, 540, 593, 598, 639, 677-688, 764, 876-877, 899-908, 1046, 1097-1098, 1249, 1286-1290, 1306, 1313, 1395, 1530-1531, 1547-1548, 1562-1563
R/tostring.R           783      66  91.57%   88, 296, 351, 421, 454, 462, 498, 555-558, 594, 658-661, 667-671, 674-677, 684-689, 772-773, 913-914, 979-986, 1036-1040, 1109, 1162, 1181-1185, 1196, 1214, 1231, 1246, 1344, 1385, 1430, 1516, 1555, 1609, 1616
R/utils.R                3       0  100.00%
R/zzz.R                 17       6  64.71%   28-33
TOTAL                 2943     217  92.63%

Diff against main

Filename        Stmts    Miss  Cover
------------  -------  ------  -------
R/generics.R       +1       0  +0.08%
TOTAL              +1       0  +0.00%

Results for commit: 4d6ca86c09f9fce6882b1dcd1df4a46f1ba7f8da

Minimum allowed coverage is 80%

:recycle: This comment has been updated with latest results

gmbecker commented 2 months ago

@averissimo @Melkiades do you have reproducible cod ethat causes this? rtables gets through all its tests and vignettes for me without ever raising this error

averissimo commented 2 months ago

@gmbecker, here:

formatters::nlines("01234567890", max_width = 5)
#> Error in FUN(X[[i]], ...): argument "fontspec" is missing, with no default

^{Created on 2024-06-10 with reprex v2.1.0}

averissimo commented 2 months ago

The problem itself is not with a direct call like this one. But some packages' CI have started to fail on building vignettes or tests and I tracked the problem to this function.

Here is a run where the problem was triggered by {rlistings} and the trace is available.

Others runs were failing from {rtables} (traceback also available)

gmbecker commented 2 months ago

Again, I really think this is a version mismatch due to improper dependency caching in your automation cc @cicdguy

the current state of main on teal.reporter passes check for me locally:

gbecker@Gabriels-MacBook-Pro nestpkgs % R CMD check teal.reporter_0.3.1.9008.tar.gz
* using log directory ‘/Users/gbecker/gabe/checkedout/nestpkgs/teal.reporter.Rcheck’
* using R version 4.3.2 (2023-10-31)
* using platform: aarch64-apple-darwin20 (64-bit)
* R was compiled by
    Apple clang version 14.0.0 (clang-1400.0.29.202)
    GNU Fortran (GCC) 12.2.0
<~snip~>
* checking for unstated dependencies in examples ... OK
* checking installed files from ‘inst/doc’ ... OK
* checking files in ‘vignettes’ ... NOTE
Files named as vignettes but with no recognized vignette engine:
   ‘vignettes/_setup.Rmd’
(Is a VignetteBuilder field missing?)
* checking examples ... OK
* checking for unstated dependencies in ‘tests’ ... OK
* checking tests ...
  Running ‘testthat.R’
 OK
* checking for unstated dependencies in vignettes ... OK
* checking package vignettes in ‘inst/doc’ ... OK
* checking running R code from vignettes ...
  ‘previewerReporter.Rmd’ using ‘UTF-8’... OK
  ‘simpleReporter.Rmd’ using ‘UTF-8’... OK
  ‘teal-reporter-blocks-overview.Rmd’ using ‘UTF-8’... OK
  ‘teal-reporter.Rmd’ using ‘UTF-8’... OK
 NONE
* checking re-building of vignette outputs ... OK
* checking PDF version of manual ... OK
* DONE

Status: 1 NOTE
See
  ‘/Users/gbecker/gabe/checkedout/nestpkgs/teal.reporter.Rcheck/00check.log’
for details.

gbecker@Gabriels-MacBook-Pro nestpkgs % R -e "library(rtables); sessionInfo()" --vanilla
<~snip~>

> library(rtables); sessionInfo()
Loading required package: formatters
Loading required package: magrittr

Attaching package: ‘rtables’

The following object is masked from ‘package:utils’:

    str

R version 4.3.2 (2023-10-31)
Platform: aarch64-apple-darwin20 (64-bit)
Running under: macOS Sonoma 14.1

Matrix products: default
BLAS:   /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/lib/libRblas.0.dylib 
LAPACK: /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/lib/libRlapack.dylib;  LAPACK version 3.11.0

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

time zone: America/Los_Angeles
tzcode source: internal

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

other attached packages:
[1] rtables_0.6.7.9004    magrittr_2.0.3        formatters_0.5.7.9001

loaded via a namespace (and not attached):
 [1] compiler_4.3.2    backports_1.4.1   fastmap_1.1.1     cli_3.6.2        
 [5] tools_4.3.2       htmltools_0.5.8.1 grid_4.3.2        checkmate_2.3.1  
 [9] digest_0.6.34     lifecycle_1.0.4   rlang_1.1.3

This is with the version of nlines that takes fonspec without a default value:

> library(rtables); formals(nlines)
Loading required package: formatters
Loading required package: magrittr

Attaching package: ‘rtables’

The following object is masked from ‘package:utils’:

    str

$x

$colwidths
NULL

$max_width
NULL

$fontspec

$col_gap
NULL

averissimo commented 2 months ago

@gmbecker Thanks! you're right, it was a problem with versions (misconfiguration in staged.dependencies and a missed version bump in DESCRIPTION)

Is this fix still relevant? It appears to allow older versions of rtables/rlistings to play (at least partially) nicely with {formatter}

gmbecker commented 2 months ago

@gmbecker Thanks! you're right, it was a problem with versions (misconfiguration in staged.dependencies and a missed version bump in DESCRIPTION)

Is this fix still relevant? It appears to allow older versions of rtables/rlistings to play (at least partially) nicely with {formatter}

I wouldn't want to bank on that with a lot more testing, I don't think its safe enough to say its supported. In other words it may happen to work but I'd consider anything that happens with version mismatches like this undefined behavior

insightsengineering / formatters

Fixes `nlines` function #300

Pull request

Changes description

Unit Tests Summary

Unit Test Performance Difference

Code Coverage Summary

Diff against main