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

add hashing #774

Merged
merged 15 commits into from
Nov 11, 2022
Merged

add hashing #774

merged 15 commits into from
Nov 11, 2022

Conversation

mhallal1
Copy link
Collaborator

@mhallal1 mhallal1 commented Nov 9, 2022

closes #751

The dataset are being hashed using rlang::hash in:

  1. data_to_datasets when using qenv
  2. get_rcode when using chunks chunks

-calculate_hashes function has been created to be used in both places.
-the filtering is not returned in get_datasets_code now but rather in get_rcode and datasets_to_data.
-messages and warnings calls are now returned in get_datasets_code instead of character strings.

Please test with chunks and qenv examples from tmg, tmc ...

Also closes #752

@mhallal1 mhallal1 added the core label Nov 9, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2022

badge

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  -------------------------------------------
R/default_filter.R                   7       7  0.00%    17-27
R/dummy_functions.R                 74      61  17.57%   12-95
R/example_module.R                  18      18  0.00%    19-36
R/get_rcode_utils.R                 52       2  96.15%   94, 99
R/get_rcode.R                      140      54  61.43%   74, 81, 86, 209-275
R/include_css_js.R                  24       0  100.00%
R/init.R                            39      21  46.15%   173, 184-185, 238-259
R/log_app_usage.R                   38      38  0.00%    34-119
R/logging.R                         13      13  0.00%    11-28
R/module_nested_tabs.R             123       7  94.31%   57, 96, 101-102, 148, 198, 227
R/module_tabs_with_filters.R        68       1  98.53%   160
R/module_teal_with_splash.R         33       2  93.94%   62, 74
R/module_teal.R                    131      20  84.73%   49, 52, 155-156, 169-175, 181-187, 210, 240
R/modules_debugging.R               18      18  0.00%    39-58
R/modules.R                        110       9  91.82%   218, 415-440
R/reporter_previewer_module.R       12       2  83.33%   18, 22
R/show_rcode_modal.R                20      20  0.00%    17-38
R/tdata.R                           41       2  95.12%   146, 172
R/utils.R                           13       0  100.00%
R/validations.R                     62      39  37.10%   103-355
R/zzz.R                             11       7  36.36%   3-14
TOTAL                             1047     341  67.43%

Diff against main

Filename                  Stmts  Miss    Cover
----------------------  -------  ------  -------
R/get_rcode.R               -11  -       -2.81%
R/module_nested_tabs.R       +8  -       +0.40%
TOTAL                        -3  -       -0.09%

Results for commit: 0e02c7a

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2022

Unit Tests Summary

    1 files    12 suites   14s ⏱️
136 tests 136 ✔️ 0 💤 0
260 runs  260 ✔️ 0 💤 0

Results for commit 9034053.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@nikolas-burkoff nikolas-burkoff left a comment

Choose a reason for hiding this comment

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

In the NEWS we should mention not only have the hashes been moved here but that we now use rlang instead of digest (this is important if the hash algorithms produce different answers - I'm not sure they are)

@nikolas-burkoff nikolas-burkoff self-assigned this Nov 10, 2022
Copy link
Contributor

@nikolas-burkoff nikolas-burkoff left a comment

Choose a reason for hiding this comment

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

Let me know what you think about these:

NEWS.md Outdated Show resolved Hide resolved
R/module_nested_tabs.R Show resolved Hide resolved
Co-authored-by: Nikolas Burkoff <[email protected]>
Signed-off-by: Mahmoud Hallal <[email protected]>
@nikolas-burkoff
Copy link
Contributor

nikolas-burkoff commented Nov 10, 2022

  • Looking at the reporter vignette in teal adding the report adds the src:
    image

image

  • Would be cool to have some new lines here:

image

@nikolas-burkoff
Copy link
Contributor

just double check the internal roxygen mentions hashes for the funcitons which now have them

@mhallal1 mhallal1 merged commit bbaf8c9 into main Nov 11, 2022
@mhallal1 mhallal1 deleted the 751_hash_move@main branch November 11, 2022 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants