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

[Bug]: functions in data raise warnings #1352

Closed
3 tasks done
chlebowa opened this issue Sep 26, 2024 · 5 comments · Fixed by #1393
Closed
3 tasks done

[Bug]: functions in data raise warnings #1352

chlebowa opened this issue Sep 26, 2024 · 5 comments · Fixed by #1393
Assignees
Labels
bug Something isn't working core

Comments

@chlebowa
Copy link
Contributor

chlebowa commented Sep 26, 2024

What happened?

When data contains a function, a warning is logged when running init:

[WARN] 2024-09-26 11:08:10.5918 pid:6800 token:[] teal In 'rlang::hash(list(data = data, modules = modules))': 'package:teal' may not be available when loading

A similar one is logged when the app is run:

[WARN] 2024-09-26 11:48:19.3963 pid:21388 token:[4e453302] teal In 'rlang::hash(data[[dataname]])': 'package:teal' may not be available when loading  

It looks like it's happening in create_app_id. I modified the function so that data is also passed through defunction and that removed the warning raised when running init but not the one raised when runninng the app.

example app
devtools::load_all("teal")

options("teal.bs_theme" = bslib::bs_theme(version = "5"))

data <- teal_data() |>
  within(iris <- iris) |>
  within(foo <- function(x) cat("hello\n"))
modules <- modules(
  example_module()
)

app <- init(data = data, modules = modules)

runApp(app)

sessionInfo()

No response

Relevant log output

> packageVersion("teal")
[1] '0.15.2.9064'

Code of Conduct

  • I agree to follow this project's Code of Conduct.

Contribution Guidelines

  • I agree to follow this project's Contribution Guidelines.

Security Policy

  • I agree to follow this project's Security Policy.
@chlebowa chlebowa added the bug Something isn't working label Sep 26, 2024
@chlebowa
Copy link
Contributor Author

chlebowa commented Sep 26, 2024

Update: The second warning occurs in .get_hashes_code.

@donyunardi
Copy link
Contributor

donyunardi commented Sep 27, 2024

With the improvements that we've done here, we could prevent the function object to be returned from the environment by prefixing the object's name with a dot (.).

Once I changed foo to .foo, I no longer see the warning.

The function object will still be in Show R Code for reproducibility.

@chlebowa
Copy link
Contributor Author

I hadn't noticed the change to the rules. I can't say I like it but I'll accept it.

However, if one leaves a function in on purpose, one will get the warnings so the problem remains.

@donyunardi
Copy link
Contributor

However, if one leaves a function in on purpose, one will get the warnings so the problem remains.

Fair point and I agree.
Thanks for raising the PR, we'll look into this.

@gogonzo
Copy link
Contributor

gogonzo commented Oct 15, 2024

Acceptance criteria:

  • filter panel should ignore objects which are clearly unfilterable: currently all objects except data.frame, matrix, MAE
  • summary table should list all datanames - filterable should have obs/subjects while unfilterable probably should be hidden in collapsible panel under summary-table.

@gogonzo gogonzo self-assigned this Oct 17, 2024
@gogonzo gogonzo linked a pull request Oct 22, 2024 that will close this issue
averissimo added a commit to insightsengineering/teal.slice that referenced this issue 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]>
vedhav added a commit that referenced this issue Nov 6, 2024
Closes #1352 

This PR enables including any data type in the `data` (`teal_data`)
object.
- unfilterable datasets (not data.frame nor MAE) are not included in the
filter-panel, but they are preserved in the `data`
- unsupported data types are displayed in the data-summary-table but
they are hidden by default
- if any unsupported dataset is in the data they data-summary displays
"show/hide unsupported" to toggle rows containing unsupported
- functions are excluded from a hash calculation and this code is not
included in SRC
<table>
<thead>
<th>hide unsupported</th>
<th>show unsupported</th>
</thead>
<tbody>
<tr>
<td>
<img width="362" alt="image"
src="https://github.com/user-attachments/assets/46839bad-b193-4b24-a549-d9f99b0e6064">
</td>
<td>
<img width="370" alt="image"
src="https://github.com/user-attachments/assets/270a077b-4175-4967-b6e4-026c8f9ac28c">

</td>
</tr>
</tbody>
<table>


<details>
<summary>App example</summary>

```r
devtools::load_all("teal.slice")
devtools::load_all("teal")
options("teal.bs_theme" = bslib::bs_theme(version = "5"))

data <- teal_data() |>
  within({
    library(MultiAssayExperiment)
    data(miniACC, envir = environment())
    iris <- iris
    foo <- function(x) cat("hello\n")
    vector <- letters
  })

modules <- modules(
  example_module(
    transformers = teal_transform_module(server = function(id, data) {
      moduleServer(id, function(input, output, session) {
        reactive({
          data() |> within({
            foo2 <- function() NULL
          })
        })
      })
    })
  ),
  example_module(datanames = "iris")
)

app <- init(data = data, modules = modules)

runApp(app)

```

</details>

---------

Signed-off-by: Vedha Viyash <[email protected]>
Co-authored-by: Marcin <[email protected]>
Co-authored-by: vedhav <[email protected]>
Co-authored-by: Vedha Viyash <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core
Projects
None yet
3 participants