insightsengineering / teal.data

Data model for teal applications
https://insightsengineering.github.io/teal.data/
Other
8 stars 7 forks source link

do not duplicate verification warning message, if it already existed in `@code` #322

Closed m7pr closed 1 month ago

m7pr commented 1 month ago

Fixes https://github.com/insightsengineering/teal/issues/1286

Previously, a verification warning was prepended to the @code slot each time a get_code(teal_data()) was run. And if a warning already existed in the code, then it was duplicated.

x <- teal_data(iris = iris)
get_code(x)
> [1] "warning('Code was not verified for reproducibility.')"
x@code <- get_code(x)
get_code(x)
> [1] "warning('Code was not verified for reproducibility.')\nwarning('Code was not verified for reproducibility.')"

The @code slot gets overwritten for teal apps, when hashes are added to the code (and sometimes libraries)

stopifnot(rlang::hash(iris) == "34844aba7bde36f5a34f6d8e39803508")
stopifnot(rlang::hash(mtcars) == "d0487363db4e6cc64fdb740cb6617fc0")
iris_raw <- iris
mtcars_raw <- mtcars

and also during this operation the warning is added to the @code and then on another get_code() you see the warning appearing twice

image

So this PR prevents that and do not add warning again if it was already in the @code

x <- teal_data(iris = iris)
get_code(x)
> [1] "warning('Code was not verified for reproducibility.')"
x@code <- get_code(x)
get_code(x)
> [1] "warning('Code was not verified for reproducibility.')"
image

Tests

> devtools::test()
ℹ Testing teal.data
✔ | F W  S  OK | Context
✔ |          2 | cdisc_data                                           
✔ |          6 | cdisc_join_keys                                      
✔ |         17 | datanames                                            
✔ |         23 | formatters_var_labels                                
✔ |      2  55 | get_code                                             
✔ |         27 | join_key                                             
✔ |         12 | join_keys-c                                          
✔ |         57 | join_keys-extract                                    
✔ |          2 | join_keys-names                                      
✔ |         17 | join_keys-parents                                    
✔ |          5 | join_keys-print                                      
✔ |         23 | join_keys                                            
✔ |         20 | teal_data                                            
✔ |          8 | verify                                               

══ Results ═══════════════════════════════════════════════════════════
Duration: 3.1 s

── Skipped tests (2) ─────────────────────────────────────────────────
• This is not urgent and can be ommitted with @linksto tag. (1):
  test-get_code.R:478:3
• We will not resolve this, as this requires code evaluation. (1):
  test-get_code.R:270:3

[ FAIL 0 | WARN 0 | SKIP 2 | PASS 274 ]
m7pr commented 1 month ago

Converting to draft, and making one more check

m7pr commented 1 month ago

Looks like might have been fixed by the removal of those lines https://github.com/insightsengineering/teal/pull/1302/files#diff-60bef5af43a1bd541299ce9bceeb226afb23deaa5e637b145c85e05ffd552b0cL72-L73 and those https://github.com/insightsengineering/teal/pull/1302/files#diff-e8f18436c76b1f6580b3b5c16b832c7a8149201e6d06dfc9c6d750b2fdbde2fcL62-L75

gogonzo commented 1 month ago

fixed in the source of the problem