Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

210 substitute teal.data::get_code with teal.code::get_code #1388

Merged
merged 6 commits into from
Oct 23, 2024

Conversation

m7pr
Copy link
Contributor

@m7pr m7pr commented Oct 18, 2024

Partner to

Replace teal.data::get_code() (that worked on teal_data()) with teal.code::get_code() (that works on qenv).

Copy link
Contributor

github-actions bot commented Oct 18, 2024

badge

Code Coverage Summary

Filename                          Stmts    Miss  Cover    Missing
------------------------------  -------  ------  -------  ----------------------------------------------------------------------------------------------------------------------------------------
R/checkmate.R                        24       0  100.00%
R/dummy_functions.R                  47      11  76.60%   27, 29, 41, 52-59
R/get_rcode_utils.R                  12       0  100.00%
R/include_css_js.R                   22      17  22.73%   12-38, 76-82
R/init.R                            108      50  53.70%   107-114, 160-169, 171, 183-204, 229-232, 239-245, 248-249, 251
R/landing_popup_module.R             25      25  0.00%    61-87
R/module_bookmark_manager.R         158     127  19.62%   47-68, 88-138, 143-144, 156, 203, 238-315
R/module_data_summary.R             189      68  64.02%   24-52, 93, 187, 227-267
R/module_filter_data.R               64       2  96.88%   22-23
R/module_filter_manager.R           230      57  75.22%   56-62, 73-82, 90-95, 108-112, 117-118, 291-314, 340, 367, 379, 386-387
R/module_init_data.R                 68       0  100.00%
R/module_nested_tabs.R              236      91  61.44%   40-142, 174, 199-201, 320, 360
R/module_snapshot_manager.R         216     146  32.41%   89-95, 104-113, 121-133, 152-153, 170-180, 184-199, 201-208, 215-230, 234-238, 240-246, 249-262, 265-273, 304-318, 321-332, 335-341, 355
R/module_teal_data.R                152      10  93.42%   41-48, 84, 135-136
R/module_teal_lockfile.R            131      44  66.41%   32-36, 44-56, 59-61, 75, 85-87, 99-101, 109-118, 121, 123, 125-126, 160-161
R/module_teal_with_splash.R          12      12  0.00%    22-38
R/module_teal.R                     190      87  54.21%   48-143, 158, 184-185, 216
R/module_transform_data.R            54      32  40.74%   17-52
R/modules.R                         181      32  82.32%   173-177, 232-235, 333-334, 366-380, 418, 430-438
R/reporter_previewer_module.R        19       2  89.47%   30, 34
R/show_rcode_modal.R                 24      24  0.00%    17-42
R/tdata.R                            14      14  0.00%    19-61
R/teal_data_module-eval_code.R       24       0  100.00%
R/teal_data_module-within.R           7       0  100.00%
R/teal_data_module.R                 58       0  100.00%
R/teal_data_utils.R                  32       0  100.00%
R/teal_reporter.R                    68       6  91.18%   69, 77, 125-126, 129, 146
R/teal_slices-store.R                29       0  100.00%
R/teal_slices.R                      63       0  100.00%
R/TealAppDriver.R                   353     353  0.00%    52-735
R/utils.R                           203       0  100.00%
R/validate_inputs.R                  32       0  100.00%
R/validations.R                      58      37  36.21%   110-377
R/zzz.R                              15      11  26.67%   4-18
TOTAL                              3118    1258  59.65%

Diff against main

Filename      Stmts    Miss  Cover
----------  -------  ------  --------
TOTAL             0       0  +100.00%

Results for commit: eb5aed1

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Oct 18, 2024

Unit Tests Summary

  1 files   25 suites   8m 29s ⏱️
255 tests 251 ✅ 4 💤 0 ❌
512 runs  508 ✅ 4 💤 0 ❌

Results for commit eb5aed1.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Oct 18, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
shinytest2-init 💔 $26.87$ $+1.01$ $0$ $0$ $0$ $0$
shinytest2-landing_popup 💔 $44.68$ $+1.32$ $0$ $0$ $0$ $0$

Results for commit d7e2367

♻️ This comment has been updated with latest results.

@gogonzo gogonzo self-assigned this Oct 18, 2024
Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sceptic about class(data) <- "qenv" to force usage of the teal.code method. What is the reason? Is it to avoid deprecation warning create teal.data::get_code?

@m7pr
Copy link
Contributor Author

m7pr commented Oct 21, 2024

@gogonzo teal.code::get_code does not have a method for teal_data. It has method only for qenv. So if you apply teal.code::get_code on teal_data then a generic is applied. Generic doesn't extract code. So you need to change the class of the obeject to qenv

@gogonzo
Copy link
Contributor

gogonzo commented Oct 21, 2024

@gogonzo teal.code::get_code does not have a method for teal_data. It has method only for qenv

Yes and not.

teal_data() |> inherits("qenv")
# TRUE

Generic doesn't extract code

No, it doesn't. Generic selects a method which matches the x class and calls this method - which technically means that it will pick the get_code() method for teal.data. You can put the print("hello") in teal.data::get_code and run this.

teal_data() |> teal.code::get_code()
# [1] "hello"

@m7pr
Copy link
Contributor Author

m7pr commented Oct 22, 2024

My bad! You are right. I got confused. Reverted the change.
Now it's only a change from teal.data::get_code -> teal.code::get_code

@m7pr m7pr requested a review from gogonzo October 22, 2024 10:26
Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

m7pr added a commit to insightsengineering/teal.code that referenced this pull request Oct 22, 2024
BLOCKED by 
- insightsengineering/teal.data#340

Closes 
- #210 

Companion to 
- insightsengineering/teal.data#343 
- insightsengineering/teal#1388

# Description

Brings `names` parameter to `get_code` so that you can limit returned
code to specific objects (and the lines that create those objects).
`get_code_dependency` was moved from `teal.data`

# Tested with

``` r
library(teal.code)

# EVAL CODE
q <- qenv()
q <- eval_code(q, code = c("a <- 1", "b <- 2"))
q@code
#> [1] "a <- 1\nb <- 2"
get_code(q, names = "a")
#> [1] "a <- 1"

# WITHIN
q <- qenv()
q <- within(q, {a <- 1; b <- 2})
q@code
#> [1] "a <- 1\nb <- 2"
get_code(q, names = "a")
#> [1] "a <- 1"

# OLD TEAL.DATA
t <- teal.data::teal_data(a = 5, code = c("a <- 1", "b <- 2"))
t@code
#> [1] "a <- 1\nb <- 2"
teal.data::get_code(t, datanames = 'a')
#> Warning in .local(object, deparse, ...): get_code(datanames) was deprecated in
#> teal.data 0.6.1, use get_code(names) instead.
#> [1] "a <- 1"
```

<sup>Created on 2024-10-16 with [reprex
v2.1.1](https://reprex.tidyverse.org)</sup>

# Local tests

```r
> devtools::test()
ℹ Testing teal.code
✔ | F W  S  OK | Context
✔ |         12 | qenv_concat                                                 
✔ |          8 | qenv_constructor                                            
✔ |         26 | qenv_eval_code                                              
✔ |      2  60 | qenv_get_code [1.1s]                                        
✔ |         10 | qenv_get_var                                                
✔ |          7 | qenv_get_warnings                                           
✔ |         40 | qenv_join                                                   
✔ |         14 | qenv_within                                                 
✔ |         12 | utils                                                       

══ Results ══════════════════════════════════════════════════════════════════
Duration: 2.2 s

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

[ FAIL 0 | WARN 0 | SKIP 2 | PASS 189 ]
````

# Local R CMD CHECK

```r
── R CMD check results ──────────────────────────────────── teal.code 0.5.0.9010 ────
Duration: 33.5s

❯ checking for future file timestamps ... NOTE
  unable to verify current time

0 errors ✔ | 0 warnings ✔ | 1 note ✖

R CMD check succeeded
```

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com>
m7pr added a commit to insightsengineering/teal.data that referenced this pull request Oct 22, 2024
Part of 
- insightsengineering/teal.code#210

Companion to
- insightsengineering/teal#1388
- insightsengineering/teal.code#214

# Description

Deprecates the usage of `datanames` parameter in
`teal.data::get_code(datanames)` in favour of
`teal.code::get_code(names)`.

VBUMP package version locally to something greater than `0.6.1` during
testing.

``` r
library(teal.data)
#> Loading required package: teal.code
tdata1 <- within(teal_data(), {a <- 1; b <- 2})
get_code(tdata1)
#> [1] "a <- 1\nb <- 2"
get_code(tdata1, datanames = "a")
#> Warning: The `datanames` argument of `get_code()` is deprecated as of teal.data 0.6.1.
#> ℹ Please use the `names` argument of `teal.code::get_code()` instead.
#> ℹ The deprecated feature was likely used in the teal.data package.
#>   Please report the issue at
#>   <https://github.com/insightsengineering/teal.data/issues>.
#> This warning is displayed once every 8 hours.
#> Call `lifecycle::last_lifecycle_warnings()` to see where this warning was
#> generated.
#> [1] "a <- 1"
```

<sup>Created on 2024-10-17 with [reprex
v2.1.1](https://reprex.tidyverse.org)</sup>

---------

Signed-off-by: Marcin <[email protected]>
Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Pawel Rucki <[email protected]>
@gogonzo
Copy link
Contributor

gogonzo commented Oct 22, 2024

CI/CD pulls released versions. Probably after replacing staged.dependencies action insightsengineering/nestdevs-tasks#65

image

@m7pr I think you need to bump teal.code version. You'll need to do the same in merged teal.data.

@m7pr m7pr merged commit 6895e1d into main Oct 23, 2024
28 checks passed
@m7pr m7pr deleted the 218_deprecate_get_code@main branch October 23, 2024 08:44
@github-actions github-actions bot locked and limited conversation to collaborators Oct 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Implement names argument in get_code to subset code for specific objects.
2 participants