182 introduce @verified slot in teal_data class by m7pr · Pull Request #188 · insightsengineering/teal.data

m7pr · 2023-11-10T12:28:48Z

Close #182

Feature description

Added @verified slot to teal_data class.
- When object is initialized with empty data and code then verified is equal to TRUE.
- If data or code are not-empty then verified is set to FALSE.
Introduced verify
- S4 generic for teal_data and qenv.error classes.
- Checks if you can reproduce teal_data@env with teal_data@code.
- Sets teal_data@verified to TRUE if you can.
- Throws an error other way.
- For qenv.error throwns an error with object@message information.
Introduced get_code S4 generic for teal_data object
- datanames parameter allows to use get_code_dependency(code, names) to get the code for specific objects.
- get_code_dependency was moved from teal.code 193 bring get_code_dependency from teal.code #194
Introduced show,teal_data-method
- adds message to the print for
  - teal_data@verified = FALSE saying object is not verified
  - teal_data@verified = TRUE saying object is verified

Not included

I did not provide a mechanism to block manual change of @valid slot, as we do not have this for @code / @env anyway.

TODO

add get_code.teal_data() S3 method
rewrite is_reproducible to S4 generic with method for teal_data and for qenv.error.
keep looking for a better name than is_reproducible Name for the check field and process for `teal_data` object #189
write teal.data::get_code S4 for teal_data object
bring get_code_dependency from teal.code to teal.data 193 bring get_code_dependency from teal.code #194
improve and simplify error statements 182 introduce @verified slot in teal_data class #188 (comment)
fix documentation suggestions
extend warnings for 3 scenarios of difference between code results and objects in @env

chlebowa · 2023-11-10T13:32:22Z

Not included

I did not provide a mechanism to block manual change of @valid slot, as we do not have this for @code / @env anyway.

There is no way to prevent it if someone is persistent enough.

Question

We wanted to extend get_code() with a warning if @valid is FALSE, however I do not see get_code.teal_data method/function

You have to specify a method for the teal_data class in this package.

github-actions · 2023-11-10T13:34:32Z

Code Coverage Summary

Filename                                 Stmts    Miss  Cover    Missing
-------------------------------------  -------  ------  -------  -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
R/as_cdisc.R                                39       4  89.74%   99-102
R/Callable.R                                45       0  100.00%
R/CallableCode.R                            36       2  94.44%   26, 63
R/CallableFunction.R                        88       3  96.59%   159-161
R/CallablePythonCode.R                      58      58  0.00%    21-227
R/cdisc_data.R                              41       1  97.56%   55
R/cdisc_join_keys.R                         11      11  0.00%    17-35
R/CDISCTealDataConnector.R                  20       3  85.00%   31, 36, 49
R/CDISCTealDataset.R                        46      11  76.09%   108-115, 204-206
R/CDISCTealDatasetConnector.R               26       1  96.15%   116
R/CodeClass.R                              111       1  99.10%   157
R/data_label.R                              36      13  63.89%   33-37, 56-63, 103
R/datanames.R                               10       0  100.00%
R/deep_clone_r6.R                            9       0  100.00%
R/dummy_function.R                           5       1  80.00%   23
R/formatters_var_labels.R                   49      21  57.14%   30, 38, 43-44, 46, 53, 99, 129-151
R/get_attrs.R                                2       2  0.00%    12-45
R/get_code.R                               173      19  89.02%   87, 140-143, 196-197, 207-208, 266, 297, 333, 337, 372, 381-385
R/get_dataname.R                             5       1  80.00%   39
R/get_dataset_label.R                        3       0  100.00%
R/get_dataset.R                             13       8  38.46%   39, 55, 80-86
R/get_datasets.R                            11       3  72.73%   96, 116, 140
R/get_join_keys.R                           10       0  100.00%
R/get_key_duplicates.R                      37       7  81.08%   41-47, 54-55
R/get_keys.R                                15       7  53.33%   68-69, 126-146
R/get_raw_data.R                            24      11  54.17%   168-181
R/include_css_js.R                           9       1  88.89%   20
R/is_pulled.R                                4       0  100.00%
R/join_key.R                                32       0  100.00%
R/join_keys-c.R                             26       0  100.00%
R/join_keys-extract.R                      122       0  100.00%
R/join_keys-names.R                         15       0  100.00%
R/join_keys-parents.R                       28       0  100.00%
R/join_keys-print.R                         47       0  100.00%
R/join_keys-utils.R                         87       1  98.85%   130
R/join_keys.R                               23       0  100.00%
R/load_dataset.R                            25      18  28.00%   58-63, 87-186
R/MAETealDataset.R                         138      57  58.70%   53, 115, 153-208, 224-229, 236-245, 282, 323-339
R/mutate_dataset.R                          18       0  100.00%
R/set_args.R                                10       5  50.00%   34-38
R/teal_data-class.R                         25       1  96.00%   70
R/teal_data-get_code.R                      14      13  7.14%    31-48
R/teal_data-show.R                           4       4  0.00%    13-18
R/teal_data.R                               39       2  94.87%   32, 59
R/TealData.R                               233     114  51.07%   183, 210, 222-291, 335-342, 375-380, 382, 384-389, 391, 408-453
R/TealDataAbstract.R                       232      24  89.66%   72, 85-88, 97-106, 215-218, 429, 454-458, 480, 486
R/TealDataConnection.R                     297     180  39.39%   58-59, 64, 67, 70, 106-163, 183, 186-188, 194-200, 205-207, 233, 238, 254-277, 287, 300, 321, 325-330, 333-336, 358-360, 364-371, 374-377, 392-406, 425-426, 446-517, 535-543, 545, 549-564, 567-570, 602, 608-612, 626, 661-663, 672-674
R/TealDataConnector.R                      196     102  47.96%   178, 190, 194, 207, 210-219, 221, 229-238, 321-325, 383-488
R/TealDataset.R                            367      23  93.73%   141-151, 382-386, 442-451, 503
R/TealDatasetConnector_constructors.R      270      52  80.74%   177-214, 727-732, 930-1006
R/TealDatasetConnector.R                   326      90  72.39%   169, 237, 251, 256, 270, 433, 456-495, 525, 540-570, 660, 670, 679-686, 699, 714-741
R/testhat-helpers.R                         54       0  100.00%
R/to_relational_data.R                      57       8  85.96%   34-35, 39, 98, 105, 111, 122-123
R/topological_sort.R                        32       0  100.00%
R/utils-code_dependency.R                  190       7  96.32%   37, 42, 252-253, 317-320
R/utils.R                                   56       9  83.93%   22-23, 27, 76-83
R/validate_data_args.R                      32       0  100.00%
R/verify.R                                  32       6  81.25%   44, 70-74
R/zzz.R                                     11      11  0.00%    4-19
TOTAL                                     3974     916  76.95%

Diff against main

Filename                  Stmts    Miss  Cover
----------------------  -------  ------  --------
R/datanames.R                -1       0  +100.00%
R/teal_data-class.R          +2       0  +0.35%
R/teal_data-get_code.R      +14     +13  +7.14%
R/teal_data-show.R           +4      +4  +100.00%
R/verify.R                  +32      +6  +81.25%
TOTAL                       +51     +23  -0.29%

Results for commit: 1735ea8

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Co-authored-by: Aleksander Chlebowski <114988527+chlebowa@users.noreply.github.com> Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>

m7pr · 2023-11-10T14:04:22Z

You have to specify a method for the teal_data class in this package.

Now it makes total sense

github-actions · 2023-11-10T14:09:37Z

Unit Tests Summary

      1 files     38 suites 15s ⏱️
  474 tests   473 ✔️ 1 💤 0 ❌
1 097 runs 1 096 ✔️ 1 💤 0 ❌

Results for commit 1735ea8.

♻️ This comment has been updated with latest results.

chlebowa · 2023-11-10T14:26:52Z

I wonder if raising an error is the way to go here. The function has to be called with reassignment anyway since the valid object has to be passed to init, so td <- validate(td) and then init(data = td, .... What if we return qenv.error? init will fail nicely, we could add a pretty message in init.

But that would be coupling the two packages. On the other hand teal Depends on teal.data anyway.

Then again, qenv.error only arises when evaluation in qenv fails, so maybe it should not be created when the code evaluates properly.

…ghtsengineering/teal.data into 182_validate_teal_data@main

Co-authored-by: Aleksander Chlebowski <114988527+chlebowa@users.noreply.github.com> Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>

…ghtsengineering/teal.data into 182_validate_teal_data@main

m7pr · 2023-11-13T11:51:51Z

@chlebowa about your last comment - for sure this is something we can consider and discuss. I think I need a bit more time to come up with my opinion on that, but will keep this in mind.

I wonder if raising an error is the way to go here. The function has to be called with reassignment anyway since the valid object has to be passed to init, so td <- validate(td) and then init(data = td, .... What if we return qenv.error? init will fail nicely, we could add a pretty message in init.

But that would be coupling the two packages. On the other hand teal Depends on teal.data anyway.

Then again, qenv.error only arises when evaluation in qenv fails, so maybe it should not be created when the code evaluates properly.

m7pr · 2023-11-22T12:11:57Z

Lastly, vignettes fail (preprocessing-delayed-data.Rmd) becuase

Error in (function (classes, fdef, mtable) :
unable to find an inherited method for function ‘get_code’ for signature ‘"CDISCTealDatasetConnector"’

library(teal.data)
library(magrittr)
adsl <- cdisc_dataset_connector(
  dataname = "ADSL",
  pull_callable = adsl_cf,
  keys = get_cdisc_keys("ADSL")
) %>%
  mutate_dataset("ADSL$SEX <- as.factor(ADSL$SEX)")
  get_code(adsl)

gogonzo · 2023-11-22T12:25:00Z

Lastly, vignettes fail (preprocessing-delayed-data.Rmd) becuase

Please remove get_code from the vignette. Not worth your time to fix this

chlebowa

Some more polish, almost there.

Fix failing min verdepcheck test: ``` * checking tests ... Running ‘testthat.R’ ERROR Running the tests in ‘tests/testthat.R’ failed. Last 13 lines of output: ══ Failed tests ════════════════════════════════════════════════════════════════ ── Error (test-datanames.R:23:3): only names of existing variables are accepted ── Error: 'expect_no_error' is not an exported object from 'namespace:testthat' ── Error (test-datanames.R:30:3): datanames supports qenv.error class ────────── Error: 'expect_no_error' is not an exported object from 'namespace:testthat' ── Error (test-teal_data.R:88:3): teal_data accepts any data provided as named list ── Error: 'expect_no_error' is not an exported object from 'namespace:testthat' ── Error (test-teal_data.R:92:3): teal_data accepts code as character ────────── Error: 'expect_no_error' is not an exported object from 'namespace:testthat' ── Error (test-teal_data.R:101:3): teal_data accepts code as language ────────── Error: 'expect_no_error' is not an exported object from 'namespace:testthat' [ FAIL 5 | WARN 0 | SKIP 1 | PASS 1132 ] Error: Test failures Execution halted ``` Signed-off-by: Pawel Rucki <12943682+pawelru@users.noreply.github.com>

… verify

…ghtsengineering/teal.data into 182_validate_teal_data@main

…urn information

…ghtsengineering/teal.data into 182_validate_teal_data@main

Signed-off-by: Aleksander Chlebowski <114988527+chlebowa@users.noreply.github.com>

chlebowa

👏 👏 👏

…FALSE

Will be introduced in `teal.data` directly insightsengineering/teal.data#188 for `teal_data` class

Close #168 Needed for insightsengineering/teal.data#188 and potential other future methods that will have more parameters than current default S4 generic of `get_code`. --------- Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com> Co-authored-by: Aleksander Chlebowski <114988527+chlebowa@users.noreply.github.com> Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com>

introduce @Valid slot in teal_data class

909faf5

m7pr added the core label Nov 10, 2023

m7pr added 7 commits November 10, 2023 13:31

improve statement for data validation in valid value

d0dde86

add tests for @Valid slot in teal_data

3ab0bb9

proposition of is_reproducible

91aec70

update documentation

e4bdfa3

change format of is_reproducible returned values

e55763d

change format of is_reproducible returned values

8b4edf1

remove comments

c7e758d

m7pr marked this pull request as ready for review November 10, 2023 13:28

[skip actions] Roxygen Man Pages Auto Update

937991a

chlebowa suggested changes Nov 10, 2023

View reviewed changes

m7pr and others added 2 commits November 10, 2023 15:01

Update R/teal_data-class.R

782c7c9

Co-authored-by: Aleksander Chlebowski <114988527+chlebowa@users.noreply.github.com> Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>

Update R/is_reproducible.R

a860997

Co-authored-by: Aleksander Chlebowski <114988527+chlebowa@users.noreply.github.com> Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>

m7pr and others added 8 commits November 13, 2023 11:58

return teal_data

77e7e04

Merge branch '182_validate_teal_data@main' of https://github.com/insi…

89f4460

…ghtsengineering/teal.data into 182_validate_teal_data@main

Empty-Commit

5fb35e8

remove hashes from reproducibility check

f189969

extend documentation of is_reproducible

38d2b8d

Update R/teal_data-class.R

f74d465

Co-authored-by: Aleksander Chlebowski <114988527+chlebowa@users.noreply.github.com> Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>

change valid slot documentation

ba00f37

Merge branch '182_validate_teal_data@main' of https://github.com/insi…

37a8f80

…ghtsengineering/teal.data into 182_validate_teal_data@main

m7pr and others added 2 commits November 13, 2023 13:12

introduce get_code.teal_data

a1658a3

Merge a1658a3 into 9aae01c

e6e2298

chlebowa suggested changes Nov 22, 2023

View reviewed changes

Comment thread R/verify.R Outdated

Comment thread R/verify.R Outdated

pawelru and others added 10 commits November 22, 2023 14:44

[skip actions] Bump version to 0.3.0.9012

b1b19e4

extend case for informing about missalignment between code and env in…

a1c5dbf

… verify

Merge a1c5dbf into b1b19e4

91c6120

[skip actions] Restyle files

020b493

rename new_teal_data to y, so its easier to read code for x and y

95d0396

Merge branch '182_validate_teal_data@main' of https://github.com/insi…

295104f

…ghtsengineering/teal.data into 182_validate_teal_data@main

[skip actions] Roxygen Man Pages Auto Update

ebbab0d

change returned content to details part in verify and pass simple ret…

7b96519

…urn information

Merge branch '182_validate_teal_data@main' of https://github.com/insi…

4957a52

…ghtsengineering/teal.data into 182_validate_teal_data@main

m7pr requested a review from chlebowa November 22, 2023 14:17

bump teal.code in DESCRIPTION

884a123

chlebowa reviewed Nov 22, 2023

View reviewed changes

Comment thread R/verify.R Outdated

chlebowa and others added 3 commits November 22, 2023 15:26

separate Description from Details in verify docs

9d06171

Signed-off-by: Aleksander Chlebowski <114988527+chlebowa@users.noreply.github.com>

Merge 9d06171 into b1b19e4

21fd65c

[skip actions] Restyle files

ea27049

chlebowa approved these changes Nov 22, 2023

View reviewed changes

Comment thread R/verify.R

gogonzo changed the base branch from main to refactor November 22, 2023 15:33

m7pr and others added 3 commits November 22, 2023 16:44

append the name of the object to verify on the warning + set call. = …

84c16c7

…FALSE

Merge branch 'refactor' into 182_validate_teal_data@main

6ecceb0

postmerge fixes

98cd064

m7pr merged commit 7ff745c into refactor Nov 22, 2023

m7pr deleted the 182_validate_teal_data@main branch November 22, 2023 16:01

m7pr mentioned this pull request Nov 22, 2023

[Feature request] Introduce validate method for teal_data #182

Closed

chlebowa pushed a commit to insightsengineering/teal.code that referenced this pull request Nov 30, 2023

remove get_code_dependency + friends (#167)

8d139c3

Will be introduced in `teal.data` directly insightsengineering/teal.data#188 for `teal_data` class

donyunardi mentioned this pull request May 28, 2025

[Feature Request]: should verify() return a object? #368

Open

3 tasks

Uh oh!

Uh oh!

Conversation

m7pr commented Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Feature description

Not included

TODO

Uh oh!

chlebowa commented Nov 10, 2023

Not included

Question

Uh oh!

github-actions Bot commented Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Diff against main

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

m7pr commented Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Tests Summary

Uh oh!

chlebowa commented Nov 10, 2023

Uh oh!

m7pr commented Nov 13, 2023

Uh oh!

m7pr commented Nov 22, 2023

Uh oh!

gogonzo commented Nov 22, 2023

Uh oh!

chlebowa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chlebowa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

m7pr commented Nov 10, 2023 •

edited

Loading

github-actions Bot commented Nov 10, 2023 •

edited

Loading

m7pr commented Nov 10, 2023 •

edited

Loading

github-actions Bot commented Nov 10, 2023 •

edited

Loading