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 teal .raw_data #1382

Merged
merged 7 commits into from
Oct 25, 2024

Conversation

averissimo
Copy link
Contributor

@averissimo averissimo commented Oct 15, 2024

wip

Pull Request

Fixes #1366

Related:

Changes description

  • .raw_data is created and supports non-standard R names, such as:
    • %pipe%
    • assigns<-
    • ...

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.

Would be nice to add tests in test-module_teal.R to confirm that teal supports what has been just enabled in teal.data

@averissimo
Copy link
Contributor Author

I was adding those on Friday when I came across #1390 and validated this: #1389

Test added

@averissimo averissimo marked this pull request as ready for review October 21, 2024 12:14
Copy link
Contributor

github-actions bot commented Oct 21, 2024

Unit Tests Summary

  1 files   25 suites   8m 30s ⏱️
258 tests 254 ✅ 4 💤 0 ❌
516 runs  512 ✅ 4 💤 0 ❌

Results for commit eaf1694.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Oct 21, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
module_teal 💔 $60.85$ $+1.47$ $+4$ $0$ $0$ $0$
shinytest2-filter_panel 💚 $43.23$ $-1.13$ $0$ $0$ $0$ $0$
shinytest2-landing_popup 💚 $46.36$ $-1.12$ $0$ $0$ $0$ $0$
shinytest2-module_bookmark_manager 💚 $37.08$ $-1.08$ $0$ $0$ $0$ $0$
shinytest2-modules 💚 $40.06$ $-1.49$ $0$ $0$ $0$ $0$
shinytest2-reporter 💚 $69.62$ $-2.63$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
module_teal 👶 $+0.37$ _when_used_as_non_native_pipe_are_present_in_datanames_in_the_pre_processing_code
module_teal 👶 $+0.39$ are_detected_as_datanames_when_defined_as_all_
module_teal 👶 $+0.46$ are_present_in_datanames_when_used_in_pre_processing_code

Results for commit 6adcf4a

♻️ This comment has been updated with latest results.

averissimo added a commit to insightsengineering/teal.data that referenced this pull request Oct 22, 2024
# Pull Request

<!--- Replace `#nnn` with your issue link for reference. -->

Fixes insightsengineering/teal#1366

Related:

- insightsengineering/teal#1382
- insightsengineering/teal.slice#622
- #340

### Changes description

- [x] Adds support for non-standard names in code dependency
- [x] Support backtick symbols in code dependency

<details>

<summary>Reproducible code for backtick support in code parser</summary>

`%add_column%` definition is not detected

```r
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)
```

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

</details>

---------

Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com>
averissimo added a commit to insightsengineering/teal.slice that referenced this pull request Oct 25, 2024
# Pull Request

Fixes insightsengineering/teal#1366

Related:

- insightsengineering/teal#1382
- #622
- insightsengineering/teal.data#340

### Changes description

- Removed assertion on datanames that start with alphabetic character
- [x] Fix problem with JS namespace in filter panel
- [x] Fix crash when filtering using MAE (both SE and Matrix)
- [x ] ~Fix upload of snapshot file that is not compatible~
- [x] Ignore datanames that contain functions, language, expression (and
other non-data objects)
    - insightsengineering/teal#1352

---------

Signed-off-by: André Veríssimo <[email protected]>
Co-authored-by: Dawid Kałędkowski <[email protected]>
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.

Great work 👍

@averissimo averissimo merged commit 147eef3 into main Oct 25, 2024
25 checks passed
@averissimo averissimo deleted the 1366_exotic_datanames@main branch October 25, 2024 08:15
@github-actions github-actions bot locked and limited conversation to collaborators Oct 25, 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()
2 participants