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

Allow non-standard datanames in code dependency #340

Merged
merged 14 commits into from
Oct 22, 2024

Conversation

averissimo
Copy link
Contributor

@averissimo averissimo commented Oct 15, 2024

Pull Request

Fixes insightsengineering/teal#1366

Related:

Changes description

  • Adds support for non-standard names in code dependency
  • Support backtick symbols in code dependency
Reproducible code for backtick support in code parser

%add_column% definition is not detected

pkgload::load_all("teal.data")
#> ℹ Loading teal.data
#> Loading required package: teal.code
td <- teal_data() |> 
  within({
    IRIS <- iris
    IRIS2 <- iris
    MTCARS <- mtcars
    `%add_column%` <- function(lhs, rhs) dplyr::bind_cols(lhs, rhs) # @
    add_column <- function(lhs, rhs) dplyr::bind_cols(lhs, rhs)
    IRIS <- IRIS %add_column% dplyr::tibble(yada = IRIS2$Species)
    IRIS <- add_column(IRIS, dplyr::tibble(yada2 = IRIS2$Species))
  })

td |> get_code(datanames = "IRIS") |> cat()
#> IRIS <- iris
#> IRIS2 <- iris
#> add_column <- function(lhs, rhs) dplyr::bind_cols(lhs, rhs)
#> IRIS <- IRIS %add_column% dplyr::tibble(yada = IRIS2$Species)
#> IRIS <- add_column(IRIS, dplyr::tibble(yada2 = IRIS2$Species))

td2 <- td |> 
  within({
    IRIS <- `%add_column%`(IRIS, dplyr::tibble(yada2 = IRIS2$Species))
  })

td2 |> get_code(datanames = "IRIS") |> cat()
#> IRIS <- iris
#> IRIS2 <- iris
#> add_column <- function(lhs, rhs) dplyr::bind_cols(lhs, rhs)
#> IRIS <- IRIS %add_column% dplyr::tibble(yada = IRIS2$Species)
#> IRIS <- add_column(IRIS, dplyr::tibble(yada2 = IRIS2$Species))
#> IRIS <- IRIS %add_column% dplyr::tibble(yada2 = IRIS2$Species)

Created on 2024-10-15 with reprex v2.1.1

@averissimo averissimo marked this pull request as ready for review October 16, 2024 17:00
Copy link
Contributor

github-actions bot commented Oct 16, 2024

badge

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  --------------------
R/cdisc_data.R                       1       0  100.00%
R/deprecated.R                      57      57  0.00%    19-344
R/dummy_function.R                   2       2  0.00%    14-15
R/formatters_var_labels.R           61       0  100.00%
R/join_key.R                        38       0  100.00%
R/join_keys-c.R                     12       0  100.00%
R/join_keys-extract.R              128       0  100.00%
R/join_keys-names.R                 15       0  100.00%
R/join_keys-parents.R               30       0  100.00%
R/join_keys-print.R                 45       0  100.00%
R/join_keys-utils.R                 73       3  95.89%   35-38
R/join_keys.R                       22       0  100.00%
R/teal_data-class.R                 26       1  96.15%   69
R/teal_data-datanames.R             20       0  100.00%
R/teal_data-get_code.R              14       0  100.00%
R/teal_data-show.R                   4       4  0.00%    14-19
R/teal_data.R                       30      16  46.67%   33, 36-42, 52-58, 61
R/testhat-helpers.R                 26       0  100.00%
R/topological_sort.R                32       0  100.00%
R/utils-get_code_dependency.R      192       1  99.48%   284
R/verify.R                          42      11  73.81%   65, 95-99, 102-106
TOTAL                              870      95  89.08%

Diff against main

Filename                         Stmts    Miss  Cover
-----------------------------  -------  ------  --------
R/teal_data-get_code.R              +2       0  +100.00%
R/utils-get_code_dependency.R       +5       0  +0.01%
TOTAL                               +7       0  +0.09%

Results for commit: c5ef914

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Oct 16, 2024

Unit Tests Summary

  1 files   14 suites   2s ⏱️
209 tests 207 ✅ 2 💤 0 ❌
283 runs  281 ✅ 2 💤 0 ❌

Results for commit c5ef914.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Oct 16, 2024

Unit Test Performance Difference

Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
get_code 👶 $+0.03$ code_can_be_retrieved_with_get_code
get_code 👶 $+0.01$ starting_with_underscore_is_detected_in_code_dependency
get_code 👶 $+0.01$ with_non_native_pipe_is_detected_code_dependency
get_code 👶 $+0.01$ with_non_native_pipe_used_as_function_is_detected_code_dependency
get_code 👶 $+0.01$ with_space_character_is_detected_in_code_dependency
get_code 👶 $+0.01$ without_special_characters_is_cleaned_and_detected_in_code_dependency

Results for commit d98c867

♻️ This comment has been updated with latest results.

@m7pr
Copy link
Contributor

m7pr commented Oct 17, 2024

Once this is ready, I will incorporate changes in teal.code as we are moving get_code_dependency in there insightsengineering/teal.code#214

@gogonzo
Copy link
Contributor

gogonzo commented Oct 22, 2024

Exotic datanames don't fail but the filter-panel for them is not displayed. There is a problem with namespace id's which are based on dataname. Specifying initial teal_slices fails an app.

app
pkgload::load_all("teal.data")
pkgload::load_all("teal.slice")
pkgload::load_all("teal")

app <- teal::init(
  data = teal_data() |> within({
    `iris raw` <- iris
    `%iris%` <- iris
  }), 
  # filter = teal_slices(
  #   teal_slice("iris raw", "Species", selected = "setosa")
  # ),
  modules = modules(
    example_module("exotic"),
    example_module("exotic-specified", datanames = "iris raw")
  )
)


runApp(app)

Skärmavbild 2024-10-22 kl  14 28 00

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.

fine here ✅

@averissimo averissimo merged commit fcad956 into main Oct 22, 2024
25 checks passed
@averissimo averissimo deleted the 1366_exotic_datanames@main branch October 22, 2024 12:50
@github-actions github-actions bot locked and limited conversation to collaborators Oct 22, 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.

[Bug]: Teal app crashes when the input teal_data has objects that fail check_simple_name()
3 participants