Coverage and devops #42
Conversation
Welcome to Codecov 🎉Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment. Thanks for integrating Codecov - We've got you covered ☂️ |
R/randomize-minimisation-pocock.R
Outdated
| # Define a custom range function | ||
| custom_range <- function(x) { | ||
| max(x, na.rm = TRUE) - min(x, na.rm = TRUE) | ||
| } |
There was a problem hiding this comment.
Przerzuciłabym to przed funkcję, skoro jest to potem wykorzystywane w tej funkcji. Wiem, że realnie to się dzieje dopiero przy wywołaniu, ale jest to bardziej intuicyjne jak widzisz to od razu a nie po przejściu przez całą funkcję.
| # sum all covariate outcomes | ||
| dplyr::summarize(total = sum(dplyr::c_across(dplyr::everything()))) |> | ||
| dplyr::pull(total) | ||
| dplyr::pull("total") |
There was a problem hiding this comment.
Przeważnie nazwy kolumn zapisujemy bez "". Tak jak w przypadku select(arm, p, total) tak w przypadku pulla. W referencjach funkcji również jest podana wersja bez cudzysłowia. Inaczej mimowolnie zastanawiamy się czy chodzi o string czy o zmienną. Ja bym tutaj te cudzysłowie zdjęła
There was a problem hiding this comment.
Jest tak ponieważ tak wymagał lintr
|
|
||
| test_that("it is enough to provide a name, an identifier, and a method id", { | ||
| conn <- pool::localCheckout(pool) | ||
| with_db_fixtures("fixtures/example_study.yml") | ||
| expect_no_error({ |
There was a problem hiding this comment.
Trochę brakuje mi w tym skrypcie przypisywania bibliotek przed funkcję, tak jak robisz to przy używaniu paczki {pool}. To zawsze zmniejsza prawdopodobieństwo, że coś się nadpisze i funkcja nie zadziała odpowiednio
There was a problem hiding this comment.
Zgadzam się, ale tak to zastałem i na razie nie zmieniałem. Docelowo fajnie byłoby w ogóle pozbyć się 4 pierwszych linijek z setup-testing-environment.R
| copy = TRUE, in_place = TRUE | ||
| ) | ||
| }) | ||
| }) | ||
|
|
||
| test_that("numerical constraints are enforced", { | ||
| conn <- pool::localCheckout(pool) | ||
| with_db_fixtures("fixtures/example_study.yml") | ||
| added_patient_id <- 1 |> as.integer() |
There was a problem hiding this comment.
W R wystarczy, że zapiszesz po prostu <- 1. Automatycznie wchodzi to jako numeric. A jesli chcesz integer konkretnie to po prostu as.integer(1), bez pipe |"|>"
There was a problem hiding this comment.
Musiał tu być integer bo domyślnie to 1 jest jako double i coś tam dalej krzyczało
There was a problem hiding this comment.
No to nadal wersja as.integer(1), bo ten pipe tutaj to trochę bez sensu
There was a problem hiding this comment.
Zastanawiam się czy jednak dokumentacja do funkcji nie powinna być przy jej definiowaniu, a nie przy wywoływaniu.
Closes: #3 #30 #32