镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

[Bug]: Functions with side effects are not detected as dependencies #278

Closed

Description

@llrs-roche

What happened?

Functions with side effects like options and set.seed are omitted on teal Show R code but they are needed to make the code reproducible:

data <- within(teal.data::teal_data(), { 
  ADSL <- teal.data::rADSL
  set.seed(1000)
  options(test = 1)
  ind <- sample(nrow(ADSL), 5)
  ADSL$STUDYID[ind] <- 1L
})

app <- teal::init(data = data, modules = teal::example_module())
runApp(app)

The code on apps will omit the second and third lines. But when user copies and pastes the code result will be different as it will use a different seed and the check step ( stopifnot(rlang::hash(ADSL) == "...) will fail.

srv_source_code() doesn't return those lines because on teal app because on the reactive module_teal_data of the module() function any code that is not a dependency of the data required is dropped (L468 of module_nested_tabs.R).

sessionInfo()

Relevant log output

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.

Activity

  1. llrs-roche commented on Nov 3, 2025

    @llrs-roche
    MemberAuthor

    The lack of dependencies is noted on ?teal.code::get_code() instead of teal_data or qenv:

    Lines where side effects occur can be flagged by adding "⁠# @linksto <object name>⁠" at the end.
    

    This issue can be closed but I suggest to add a warning if there is some common functions without the linksto comment, as this will help users be more aware of this: set.seed() and options() are the ones that come to my mind.

  2. llrs-roche commented on Nov 26, 2025

    @llrs-roche
    MemberAuthor

    This is not solved with linksto when the code of a variable is required:

    library("teal.code")
    packageVersion("teal.code")
    #> [1] '0.7.0.9002'
    q <- within(qenv(), {
      
      library(random.cdisc.data)
      
      set.seed(1) # @linksto ADSL
      ADSL <- radsl(seed = 1)
    })
    
    cat(get_code(q["ADSL"]))
    #> library(random.cdisc.data)
    #> ADSL <- radsl(seed = 1)

    Created on 2025-11-26 with reprex v2.1.1

    Note how the exported code for ADSL doesn't have the set.seed() call

  3. self-assigned this
    on Dec 3, 2025
  4. osenan commented on Dec 4, 2025

    @osenan
    Contributor

    Hi @llrs-roche the problem is in the within function.
    If you look at this code it works:

    library("teal.code")
    packageVersion("teal.code")
    #> [1] '0.7.0.9002'
    q <- eval_code(qenv(), "{
      
      library(random.cdisc.data)
      
      set.seed(10) # @linksto test
      test <- runif(1)
      }"
    )
    
    cat(get_code(q["test"]))
    

    I have to fix therefore the way within is implemented

  5. osenan commented on Dec 5, 2025

    @osenan
    Contributor

    After analyzing together with @averissimo , I understand better what is happening. I do not see an easy solution, but the current scenario can be improved.

    The problem of within is that it evaluates R expression, and R expressions only capture the structure of the code, not other details like the code style (spaces, tabs) or code comments. "@linksto" is a code comment, therefore it will always be ignored in within.

    e <- expression({
      test <- runif(1) # code comment
    })
    print(e)
    

    On the other hand, function eval_code uses string as input, which can be parsed. In eval_code we can include more information, like code comments. That is why @linksto works.

    What can be done?

    1. Improve documentation so users and teal developers are more aware of the limitations of eval_code and within to carefully choose one or another. That would mean:
      • Add more documentation and even examples to both functions to make this information clearly available
      • Set a check in within that will trigger a warning when code comments are used, or when at least when teal tags like "# @linksto" are used
      • Additionally, we could add a warning in within expressions that would be trigger if set.seed or options are called
        If required, we can also include examples and explanation in tutorials.

    This alternative is quick and will reduce the chance of error.

    1. Add operator to within to link dependent code expressions
      something like %linksto% that in a expression like
    qenv() |> 
      within({
        set.seed(2) %linktsto% a
        a <- runif(1)
      })
    

    Would capture the code dependency of a with linksto
    This would require more developing time. We also need to see if it is required or we are good using within for code expressions and eval_code for code strings.

    I think that the improvement in documentation is better.

  6. llrs-roche commented on Dec 5, 2025

    @llrs-roche
    MemberAuthor

    We have replaced on code and examples to within, pointing this now might cause some problems.
    We might still get the comment with utils::getParseData if we set option(keep.source = TRUE) inside within.
    However, I agree that more documentation around this feature/corner case would be helpful. It is only mentioned in a line.

    BTW if we improve documentation we should add the within alias for this method. This way users will find it with ?within.

  7. m7pr commented on Dec 8, 2025

    @m7pr
    Contributor

    Hi @osenan and @llrs-roche , so sorry I was off and gogonzo was off. If we were there we would immediately tell you that side-effects can only be dealt with # @linksto and eval_code. eval_code deals with character code, where within doesn't and looses comments.

    Looks like you already learnt that, the hard way. At least now you know, which is good.
    I give you my blessing to improve the documentation whenever you want : ) if only it makes the user experience better.

    Lastly, you can see that library() calls are detected, even though they don't have # @linksto tags. Maybe you can have a look on how this is done for detection of library() and do the same for options and set.seed. However, even though set.seed is often needed for some objects, it is not always needed for all objects, so I would not make this function to be defaulty extracted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingcore

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions