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

update remove_enclosing_curly_braces #98

Merged
merged 13 commits into from
Nov 25, 2022
Merged

update remove_enclosing_curly_braces #98

merged 13 commits into from
Nov 25, 2022

Conversation

nikolas-burkoff
Copy link
Contributor

@nikolas-burkoff nikolas-burkoff commented Nov 24, 2022

Closes #4

Compare this branch to main for existing module qenv's and also with for example:

library(teal.code)
library(teal.widgets)
library(shiny)


ui <- fluidPage(
  verbatim_popup_ui("err", "Error"),
  verbatim_popup_ui("ok", "OK")
)

server <- function(input, output, session) {

  q_ok <- reactive({
    new_qenv(
      list2env(list(y = 2, x = 1)),
      code = quote({
        x <- 1
        y <- 2
      })
    ) |> eval_code(quote({
      if (x == 1) {
        print("x = 1")
      }
    }))
  })


  verbatim_popup_srv(
    "ok",
    verbatim_content = reactive(get_code(q_ok())),
    title = "ok"
  )

  q_err <- reactive({
    new_qenv(
      list2env(list(y = 2)),
      code = quote({
        x <- 1
        y <- 2
      })
    ) |> eval_code(quote({
      if (x == 1) {
        print("x = 1")
      }
    }))
  })

  verbatim_popup_srv(
    "err",
    verbatim_content = reactive(get_code(q_err())),
    title = "Error"
  )

}

shinyApp(ui, server)

and

library(teal.code)
data_q <- new_qenv()
data_q <- eval_code(new_qenv(), "{iris_data <- iris}")
warning_qenv <- eval_code(
  data_q,
  bquote({p <- hist(iris_data[, .("Sepal.Length")], ff = "")})
)
cat(get_warnings(warning_qenv))

@nikolas-burkoff nikolas-burkoff changed the title update code update remove_enclosing_curly_braces Nov 24, 2022
@Polkas Polkas self-assigned this Nov 24, 2022
R/utils.R Outdated
#'
#' @return The string without curly braces
#' @return Character string without curly braces
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#' @return Character string without curly braces
#' @return `character(1)` string without curly braces.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it doesn't always return character(1) - but I have removed the upper case "C"


# converts vector of expressions to character
format_expression <- function(code) {
unlist(lapply(as.character(code), remove_enclosing_curly_braces))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not vapply with character(1)?:)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and I can't really change the output of remove_enclosing_curly_braces without changing chunks which I would rather not do...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...though I guess I could if we test to make sure chunks are still good

R/utils.R Outdated Show resolved Hide resolved
@nikolas-burkoff nikolas-burkoff marked this pull request as ready for review November 25, 2022 09:30
@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2022

Unit Tests Summary

    1 files    12 suites   3s ⏱️
112 tests 112 ✔️ 0 💤 0
443 runs  443 ✔️ 0 💤 0

Results for commit 0bb61c8.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2022

badge

Code Coverage Summary

Filename                 Stmts    Miss  Cover    Missing
---------------------  -------  ------  -------  ---------------------------------------------------------------------------------------------------------------------------------------------
R/chunk.R                  150       3  98.00%   363-369
R/chunks.R                 416      41  90.14%   334, 424, 430-433, 439, 459, 466-467, 484-491, 500, 506-515, 561, 587, 593-594, 596, 612, 621, 630, 672, 675, 699-704, 1070, 1092, 1149, 1202
R/get_eval_details.R       118     118  0.00%    17-188
R/include_css_js.R           7       7  0.00%    12-19
R/qenv-concat.R             10       0  100.00%
R/qenv-constructor.R        12       0  100.00%
R/qenv-eval_code.R          48       2  95.83%   91, 100
R/qenv-get_code.R           16       0  100.00%
R/qenv-get_var.R            17       0  100.00%
R/qenv-get_warnings.R       22       0  100.00%
R/qenv-join.R               46       0  100.00%
R/qenv-show.R                4       0  100.00%
R/utils.R                   16       0  100.00%
TOTAL                      882     171  80.61%

Diff against main

Filename      Stmts    Miss  Cover
----------  -------  ------  -------
R/utils.R        -8     -19  +79.17%
TOTAL            -8     -19  +1.96%

Results for commit: 51254db

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

@Polkas Polkas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tested with sample apps and with code

library(teal.code)
a <- new_qenv(
  list2env(list(y = 2, x = 1)),
  code = quote({
    x <- 1
    y <- 2
  })
) |> eval_code(quote({
  if (x == 1) {
    print("x = 1")
  }
})) 

b <- new_qenv(
  list2env(list(y = 2, x = 1)),
  code = quote({
    x <- 1
    y <- 2
  })
) |> eval_code(quote({
  if (x == 1) {
    print("x = 1")
  }
}))

c <- a |> eval_code(quote({
  c <- 4
}))

concat(a, b)|> 
  get_code() |> paste(collapse = "\n") |> cat()

join(a, c)|> 
  get_code() |> paste(collapse = "\n") |> cat()


a2 <- new_qenv(
  list2env(list(y = 2, x = 1)),
  code = quote({
    {
      x <- 1
      y <- 2
    }
  })
) |> eval_code(quote({
  if (x == 1) print("x = 1")
})) 

a2 |> get_code() |> paste(collapse = "\n") |> cat()

a3 <- new_qenv(
  list2env(list(y = 2, x = 1)),
  code = quote({
    x <- 1; y <- 2
  })
) |> eval_code(quote({
  if (x == 1) print("x = 1"); print("Hello")
}))

a3 |> get_code() |> paste(collapse = "\n") |> cat()

we tested some edge cases too.

@nikolas-burkoff nikolas-burkoff merged commit c8f7536 into main Nov 25, 2022
@nikolas-burkoff nikolas-burkoff deleted the 4_brackets@main branch November 25, 2022 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests to remove_enclosing_curly_braces and use for qenv?
2 participants