Skip to content

Allow non-standard datanames in teal .raw_data - #1382

Merged
averissimo merged 7 commits into
mainfrom
1366_exotic_datanames@main
Oct 25, 2024
Merged

Allow non-standard datanames in teal .raw_data#1382
averissimo merged 7 commits into
mainfrom
1366_exotic_datanames@main

Conversation

@averissimo

@averissimo averissimo commented Oct 15, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

wip

Pull Request

Fixes #1366

Related:

Changes description

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

This was referenced Oct 15, 2024
Merged
Merged
@gogonzo gogonzo self-assigned this Oct 15, 2024
gogonzo
gogonzo reviewed Oct 21, 2024
View reviewed changes

@gogonzo gogonzo left a comment

Copy link
Copy Markdown
Contributor

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

averissimo commented Oct 21, 2024

Copy link
Copy Markdown
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
@github-actions

github-actions Bot commented Oct 21, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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.

@github-actions

github-actions Bot commented Oct 21, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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
@averissimo @dependabot-preview
Allow non-standard datanames in code dependency (#340)
fcad956 
# 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
@averissimo @gogonzo
Allow non-standard datanames in filter data (#622)
dc9f82b 
# 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 <211358+averissimo@users.noreply.github.com>
Co-authored-by: Dawid Kałędkowski <dawid.kaledkowski@gmail.com>
gogonzo
gogonzo approved these changes Oct 25, 2024
View reviewed changes

@gogonzo gogonzo left a comment

Copy link
Copy Markdown
Contributor

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
@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.

Reviewers

1 more reviewer
@gogonzo gogonzo gogonzo approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@gogonzo gogonzo

Labels

core

Projects

None yet

Milestone

No milestone

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

@averissimo @gogonzo