-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
211 [.teal_data
S3 method
#346
Conversation
Code Coverage Summary
Diff against main
Results for commit: a24bac3 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 13 suites 1s ⏱️ Results for commit 967c1c5. |
Unit Tests Summary 1 files 14 suites 1s ⏱️ Results for commit a24bac3. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once I read in detail the code and documentation I have some feedback.
I think the examples will be very helpful, I am not sure if the current changes on documentation have the desired consequences.
More importantly matching base's generic argument and take advantage of the S3 methods used here will probably make developing and maintaining the code easier.
When I checked locally the examples failed (It could be my setup), but running interactively shows it too (so there might be some undeclared expectations/dependencies):
d> data["a"]
Error en get(x, envir = ns, inherits = FALSE):
objeto '[.qenv' no encontrado
d> traceback()
4: get(x, envir = ns, inherits = FALSE)
3: utils::getFromNamespace("[.qenv", "teal.code") at teal_data-extract.R#33
2: `[.teal_data`(data, "a")
1: data["a"]
d> packageVersion("teal.code")
[1] '0.5.0.9011'
d> packageVersion("teal.data")
[1] '0.6.0.9015'
Any chance you can change your local setup to English : P ?
https://stackoverflow.com/questions/16347731/how-to-change-the-locale-of-r |
@llrs-roche you need to install Thanks for the attempt! |
Unit Test Performance Difference
Additional test case details
Results for commit 9c92975 ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes. I think this is mostly ready for merging, I just added a petition to have a test for a numeric and NA_character_.
I think, the workaround with NextMethod()
come from defining it differently than the generic, but seem easy enough.
I tried loading teal.code@211_subset@main and running [.qenv exampels still got the same problems object 'parsed_code' not found
, so it might be something related to my environment.
And you did good. I actually left |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments
Co-authored-by: Dawid Kałędkowski <[email protected]> Signed-off-by: Marcin <[email protected]>
Co-authored-by: Dawid Kałędkowski <[email protected]> Signed-off-by: Marcin <[email protected]>
Feel free to merge but I still have problems with executing the code with the latest update from teal.code@211_subset@main: d> data <- teal_data()
d> data <- eval_code(data, "a <- 1;b<-2")
Error in parse(text = code_split[[i]], keep.source = FALSE) :
<text>:1:12: unexpected symbol
1: a <- 1;b<-2a
^
d> data["a"]
✅︎ verified teal_data object
<environment: 0x00000171ebb781d0> [L]
Parent: <environment: devtools_shims>
Warning message:
In `[.teal_data`(data, "a") :
None of `names` elements exist in `teal_data`. Returning empty `teal_data`.
d> data[c("a", "b")]
✅︎ verified teal_data object
<environment: 0x00000171ea8c9710> [L]
Parent: <environment: devtools_shims>
Warning message:
In `[.teal_data`(data, c("a", "b")) :
None of `names` elements exist in `teal_data`. Returning empty `teal_data`.
d>
d> session_info()
─ Session info ───────────────────────────────────────────────────────────────────────
setting value
version R version 4.4.1 (2024-06-14 ucrt)
os Windows 11 x64 (build 22631)
system x86_64, mingw32
ui RStudio
language en
collate Spanish_Spain.utf8
ctype Spanish_Spain.utf8
tz Europe/Madrid
date 2024-10-30
rstudio 2024.09.0+375 Cranberry Hibiscus (desktop)
pandoc 3.5 @ C:\\Users\\sanchosl\\AppData\\Local\\Pandoc\\pandoc.exe
─ Packages ───────────────────────────────────────────────────────────────────────────
! package * version date (UTC) lib source
backports 1.5.0 2024-05-23 [2] CRAN (R 4.4.0)
brio 1.1.5 2024-04-24 [2] CRAN (R 4.4.1)
cachem 1.1.0 2024-05-16 [2] CRAN (R 4.4.1)
callr 3.7.6 2024-03-25 [2] CRAN (R 4.4.1)
checkmate 2.3.2 2024-07-29 [2] CRAN (R 4.4.1)
cli 3.6.3 2024-06-21 [2] CRAN (R 4.4.1)
curl 5.2.3 2024-09-20 [2] CRAN (R 4.4.1)
desc 1.4.3 2023-12-10 [2] CRAN (R 4.4.1)
devtools * 2.4.5 2022-10-11 [2] CRAN (R 4.4.1)
digest 0.6.37 2024-08-19 [2] CRAN (R 4.4.1)
ellipsis 0.3.2 2021-04-29 [2] CRAN (R 4.4.1)
fastmap 1.2.0 2024-05-15 [2] CRAN (R 4.4.1)
fs 1.6.4 2024-04-25 [2] CRAN (R 4.4.1)
glue 1.8.0 2024-09-30 [2] CRAN (R 4.4.1)
htmltools 0.5.8.1 2024-04-04 [2] CRAN (R 4.4.1)
htmlwidgets 1.6.4 2023-12-06 [2] CRAN (R 4.4.1)
httpuv 1.6.15 2024-03-26 [2] CRAN (R 4.4.1)
later 1.3.2 2023-12-06 [2] CRAN (R 4.4.1)
lifecycle 1.0.4 2023-11-07 [2] CRAN (R 4.4.1)
magrittr 2.0.3 2022-03-30 [2] CRAN (R 4.4.1)
memoise 2.0.1 2021-11-26 [2] CRAN (R 4.4.1)
mime 0.12 2021-09-28 [2] CRAN (R 4.4.0)
miniUI 0.1.1.1 2018-05-18 [2] CRAN (R 4.4.1)
pkgbuild 1.4.4 2024-03-17 [2] CRAN (R 4.4.1)
pkgload 1.4.0 2024-06-28 [2] CRAN (R 4.4.1)
processx 3.8.4 2024-03-16 [2] CRAN (R 4.4.1)
profvis 0.4.0 2024-09-20 [2] CRAN (R 4.4.1)
promises 1.3.0 2024-04-05 [2] CRAN (R 4.4.1)
ps 1.8.0 2024-09-12 [2] CRAN (R 4.4.1)
purrr 1.0.2 2023-08-10 [2] CRAN (R 4.4.1)
R6 2.5.1 2021-08-19 [2] CRAN (R 4.4.1)
Rcpp 1.0.13 2024-07-17 [2] CRAN (R 4.4.1)
remotes 2.5.0 2024-03-17 [2] CRAN (R 4.4.1)
rlang 1.1.4 2024-06-04 [2] CRAN (R 4.4.1)
rprojroot 2.0.4 2023-11-05 [2] CRAN (R 4.4.1)
rstudioapi 0.17.1 2024-10-22 [2] CRAN (R 4.4.1)
sessioninfo 1.2.2 2021-12-06 [2] CRAN (R 4.4.1)
shiny 1.9.1 2024-08-01 [2] CRAN (R 4.4.1)
teal.code * 0.5.0.9012 2024-10-30 [1] Github (insightsengineering/teal.code@a32939c)
VP teal.data * 0.6.0.9015 2024-10-21 [?] Github (insightsengineering/teal.data@b5ccb1d) (on disk 0.6.0.9012)
testthat * 3.2.1.1 2024-04-14 [2] CRAN (R 4.4.1)
urlchecker 1.0.1 2021-11-30 [2] CRAN (R 4.4.1)
usethis * 3.0.0 2024-07-29 [2] CRAN (R 4.4.1)
vctrs 0.6.5 2023-12-01 [2] CRAN (R 4.4.1)
withr 3.0.1 2024-07-31 [2] CRAN (R 4.4.1)
xtable 1.8-4 2019-04-21 [2] CRAN (R 4.4.1)
[1] C:/Users/sanchosl/R-dev
[2] C:/Users/sanchosl/AppData/Local/R/win-library/4.4
[3] C:/Program Files/R/R-4.4.1/library
V ── Loaded and on-disk version mismatch.
P ── Loaded and on-disk path mismatch. |
Since insightsengineering/teal.code#216 is ready, I open this PR for the review. Let's see results of CI/CD checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 🧯
Companion to insightsengineering/teal.code#216
Introduces
[.teal_data
S3 method forteal_data
that is based on[.qenv
but also limitsjoin_keys
.