Add get sql function (#68)

* fix typo in contributing guide * start to add get_clean_sql and tests (WIP) * add code cov details to contributing file * Add link to reference documentation in the examples section * commit current progress adding unit tests (WIP) * fix typo in contributing guide * start to add get_clean_sql and tests (WIP) * add code cov details to contributing file * Add link to reference documentation in the examples section * commit current progress adding unit tests (WIP) * fix tests for sql function * add roxygen2 examples to contributing file * add connecting to sql vignette * update get clean sql, contributing and reference documentation * Increment version number to 0.2.0 * tidy package following lintr advice * remove capitalised extension test (causes test errors on linux) * add Jen as a contributor to the package * Initial response to PR comments (more to complete on troubleshooting) * fix lint issues * update connecting to sql vignette to specific select style queries only * update wordlist and rebuild package docs
dfe-analytical-services · Mar 27, 2024 · abd6e19 · abd6e19
1 parent 1ef40da
commit abd6e19
Show file tree

Hide file tree

Showing 16 changed files with 355 additions and 17 deletions.
diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md
@@ -42,12 +42,12 @@ install.packages("lintr")
 install.packages("styler")
 ```
 
-Where possible, we'd recommend following the [Test Driven Development (TDD)](https://testdriven.io/test-driven-development/) approach:
+Where possible, we'd recommend following the [Test Driven Development (TDD)](https://testdriven.io/test-driven-development/) approach. Though if you're new to package development, or already have code for a specific function feel free to start with step 2, to copy the function into the package and then go back to step 1 afterwards. 
 
-1. Write tests for the behaviour you want. Either edit an existing test script, or if adding a new function, create a test script using:
+1. Write tests using [testthat](https://r-pkgs.org/testing-basics.html) for the behaviour you want. Either edit an existing test script, or if adding a new function, create a test script using:
 
 ``` r
-usethis::usetest("name_of_new_function")
+usethis::use_test("name_of_new_function")
 ```
 
 2. Write just enough code so that the tests pass. Again, either edit an existing function, or add a new R script using:
@@ -56,38 +56,54 @@ usethis::usetest("name_of_new_function")
 usethis::use_r("name_of_new_function")
 ```
 
-3. Add documentation for what you've done. Follow the [roxygen2](https://roxygen2.r-lib.org/articles/rd.html) pattern for comments.
+3. Add documentation for what you've done. Follow the [roxygen2](https://roxygen2.r-lib.org/articles/rd.html) pattern for comments. Here's an example of what it looks like for a basic `add()` function:
+
+```
+#' @description Add together two numbers
+#'
+#' @param x A number.
+#' @param y A number.
+#' @return A number.
+#' @examples
+#' add(1, 1)
+#' add(10, 1)
+add <- function(x, y) {
+  x + y
+}
+```
 
 4. Continue to improve code while keeping tests passing. You can automatically style code using:
 
 ``` r
 styler::style_pkg()
 ```
 
-5. Run a full check of the package. Here's a few ways you can do this:
+5. Run a full check of the package using the following functions:
 
 ``` r
 devtools::check() # General package check, can also use Ctrl-Shift-E
-lintr::lint_pkg() # Check styling of code
+lintr::lint_package() # Check formatting of code
 spelling::spell_check() # Check for spelling mistakes
 ```
 
 ## Handy workflows
 
 Keyboard shortcuts for the `devtools` package to use while in RStudio:
 
-* `load_all()` (Ctrl-Shift-L): Load code with dfeR package
-* `test()` (Ctrl-Shift-T): Run tests
-* `document()` (Ctrl-Shift-D): Rebuild docs and NAMESPACE
-* `check()` (Ctrl-Shift-E): Check complete package
+``` r
+load_all() # (Ctrl-Shift-L): Load code with dfeR package
+test() # (Ctrl-Shift-T): Run tests
+document() # (Ctrl-Shift-D): Rebuild docs and NAMESPACE
+check() # (Ctrl-Shift-E): Check complete package
+```
 
 We recommend using the [usethis](https://usethis.r-lib.org/index.html) package where possible for consistency and simplicity.
 
 ## Adding package dependencies
 
 Add any packages the package users will need with:
 ``` r
-usethis::use_package(pkgname, type = "imports")
+usethis::use_package(pkgname)
 ```
 
 Add any packages that package developers only may need with:
@@ -159,7 +175,19 @@ lintr::lint_package()
 
 ### Testing
 
-We use [testthat](https://cran.r-project.org/package=testthat) for unit tests, we expect all new functions to have some level of test coverage.  
+We use [testthat](https://cran.r-project.org/package=testthat) for unit tests, we expect all new functions to have some level of test coverage. 
+
+If you want to see examples of existing tests for inspiration, take a look inside the `tests/testthat/` folder.
+
+### Test coverage
+
+There are GitHub Actions workflows that check and link the package to [codecov.io](https://app.codecov.io/gh/dfe-analytical-services/), this runs automatic scans to check the % of lines in functions that we are testing. On the [dfeR codecov pages](https://app.codecov.io/gh/dfe-analytical-services/dfeR) you can preview the variation by branch and commit to see the impact of changes made.
+
+You will need to create an account or login using GitHub to see the pages.
+
+The current % of coverage is shown as a badge on the package [README on GitHub](https://github.com/dfe-analytical-services/dfeR).
+
+It is worth noting that 100% coverage does not mean that the tests are perfect, it only means that all lines are ran in tests, so it's more a measure of quantity rather than quality. Interesting to see all the same though, and we'd recommend using it to spot any potential elements of more complicated functions that you may have forgotten to test.
 
 ### Spelling
 
@@ -179,6 +207,14 @@ To automatically pick up genuine new words in the package and add to this list,
 spelling::update_wordlist()
 ```
 
+## Adding vignettes
+
+Vignettes can be found in the `vignettes/` folder as .Rmd files. To start a new one use:
+
+``` r
+usethis::use_vignette("name_of_vignette")
+```
+
 ## Code of Conduct
 
 Please note that the dfeR project is released with a [Contributor Code of Conduct](CODE_OF_CONDUCT.md). By contributing to this project you agree to abide by its terms.
diff --git a/.gitignore b/.gitignore
@@ -4,3 +4,4 @@
 .httr-oauth
 .DS_Store
 docs
+inst/doc
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -1,11 +1,14 @@
 Type: Package
 Package: dfeR
 Title: Common DfE R tasks
-Version: 0.1.1
+Version: 0.2.0
 Authors@R: c(
     person("Cam", "Race", , "[email protected]", role = c("aut", "cre")),
     person("Laura", "Selby", , "[email protected]", role = "aut"),
-    person("Adam", "Robinson", role = "aut")
+    person("Adam", "Robinson", role = "aut"),
+    person("Jen", "Machin", , "[email protected]", role = "ctb"),
+    person("Rich", "Bielby", , "[email protected]", role = "ctb",
+           comment = c(ORCID = "0000-0001-9070-9969"))
   )
 Description: This package contains R functions to allow DfE analysts to
     re-use code for common analytical tasks that are undertaken across the
@@ -17,8 +20,12 @@ BugReports: https://github.com/dfe-analytical-services/dfeR/issues
 Imports: 
     lifecycle
 Suggests:
+    knitr,
+    rmarkdown,
     spelling,
     testthat (>= 3.0.0)
+VignetteBuilder: 
+    knitr
 Config/testthat/edition: 3
 Encoding: UTF-8
 Language: en-GB

diff --git a/NAMESPACE b/NAMESPACE
@@ -4,5 +4,6 @@ export(format_ay)
 export(format_ay_reverse)
 export(format_fy)
 export(format_fy_reverse)
+export(get_clean_sql)
 export(round_five_up)
 importFrom(lifecycle,deprecated)
diff --git a/NEWS.md b/NEWS.md
@@ -1,3 +1,18 @@
+# dfeR 0.2.0
+
+Add function for formatting financial years:
+
+- format_fy()
+
+Add reversing functions for academic and financial years:
+
+- format_ay_reverse()
+- format_fy_reverse()
+
+Add function for grabbing and cleaning a SQL script, and vignette for connecting to SQL.
+
+- get_clean_sql()
+
 # dfeR 0.1.1
 
 Add default value to decimal place argument of round_five_up() function.

diff --git a/R/get_clean_sql.R b/R/get_clean_sql.R
@@ -0,0 +1,66 @@
+#' Get a cleaned SQL script into R
+#'
+#' @description
+#' This function cleans a SQL script, ready for using within R in the DfE.
+#'
+#' @param filepath path to a SQL script
+#' @param additional_settings TRUE or FALSE boolean for the addition of
+#' settings at the start of the SQL script
+#' @return Cleaned string containing SQL query
+#' @export
+#' @examples
+#' # This assumes you have already set up a database connection
+#' # and that the filepath for the function exists
+#' # For more details see the vignette on connecting to SQL
+#'
+#' # Pull a cleaned version of the SQL file into R
+#' if (file.exists("your_script.sql")) {
+#'   sql_query <- get_clean_sql("your_script.sql")
+#' }
+get_clean_sql <- function(filepath, additional_settings = FALSE) {
+  if (!additional_settings %in% c(TRUE, FALSE)) {
+    stop(
+      "additional_settings must be either TRUE or FALSE"
+    )
+  }
+
+  # check filepath leads to a SQL file
+  if (tolower(tools::file_ext(filepath)) != "sql") {
+    stop("filepath must point to a SQL script, with a .sql extension")
+  }
+
+  # The file() function will error if the file can't be found
+  # Open a connection to the file
+  con <- file(filepath, "r")
+  sql_string <- ""
+
+  while (TRUE) {
+    line <- readLines(con, n = 1)
+
+    if (length(line) == 0) {
+      break
+    }
+
+    line <- gsub("\\t", " ", line)
+    line <- gsub("\\n", " ", line)
+
+    if (grepl("--", line) == TRUE) {
+      line <- paste(sub("--", "/*", line), "*/")
+    }
+
+    sql_string <- paste(sql_string, line)
+  }
+
+  # Close connection to the file
+  close(con)
+
+  if (additional_settings == TRUE) {
+    # Prefix with settings that sometimes help
+    sql_string <- paste(
+      "SET ANSI_PADDING OFF",
+      "SET NOCOUNT ON;"
+    )
+  }
+
+  return(sql_string)
+}
diff --git a/README.Rmd b/README.Rmd
@@ -91,3 +91,5 @@ This is a basic example showing the `format_ay()` function:
 library(dfeR)
 format_ay(202425)
 ```
+
+For more details on all the functions available in this package, and examples of how to use them, please see our [dfeR package reference documentation](https://dfe-analytical-services.github.io/dfeR/reference/index.html).
diff --git a/README.md b/README.md
@@ -104,3 +104,7 @@ library(dfeR)
 format_ay(202425)
 #> [1] "2024/25"
 ```
+
+For more details on all the functions available in this package, and
+examples of how to use them, please see our [dfeR package reference
+documentation](https://dfe-analytical-services.github.io/dfeR/reference/index.html).
diff --git a/_pkgdown.yml b/_pkgdown.yml
@@ -5,3 +5,16 @@ template:
   bslib:
     pkgdown-nav-height: 81.4468px
     code-color: "ffffff"
+
+reference:
+- title: Helper functions
+  contents:
+  - round_five_up
+
+- title: Formatting
+  contents:
+  - starts_with("format_")
+
+- title: Database connections
+  contents:
+  - get_clean_sql
diff --git a/inst/WORDLIST b/inst/WORDLIST
@@ -2,11 +2,14 @@ CMD
 Codecov
 DfE
 Lifecycle
+ORCID
+SSMS
 ay
-ch
+dbplyr
 dfeshiny
-ethz
+fy
 lauraselby
+odbc
 pkgdown
 renv
-stat
+sql
diff --git a/man/dfeR-package.Rd b/man/dfeR-package.Rd
diff --git a/man/get_clean_sql.Rd b/man/get_clean_sql.Rd
diff --git a/tests/sql_scripts/simple.sql b/tests/sql_scripts/simple.sql
@@ -0,0 +1,3 @@
+-- Simple SQL script to get all data from my database table
+
+SELECT * FROM [my_database_table]
diff --git a/tests/testthat/test-get_clean_sql.R b/tests/testthat/test-get_clean_sql.R
@@ -0,0 +1,54 @@
+test_that("Can retrieve basic script", {
+  expect_equal(
+    get_clean_sql("../sql_scripts/simple.sql"),
+    paste0(
+      " /* Simple SQL script to get all data from my database table",
+      " */  SELECT * FROM [my_database_table]"
+    )
+  )
+})
+
+test_that("Adds additional settings", {
+  # Check that the output starts with the desired lines
+  expect_true(
+    grepl(
+      "^SET ANSI_PADDING OFF SET NOCOUNT ON;",
+      get_clean_sql("../sql_scripts/simple.sql", additional_settings = TRUE)
+    )
+  )
+})
+
+test_that("Doesn't add additional settings", {
+  # Check that the output doesn't start with the additional lines
+  expect_false(
+    grepl(
+      "^SET ANSI_PADDING OFF SET NOCOUNT ON;",
+      get_clean_sql("../sql_scripts/simple.sql", additional_settings = FALSE)
+    )
+  )
+  # Check that the output doesn't start with the additional lines
+  expect_false(
+    grepl(
+      "^SET ANSI_PADDING OFF SET NOCOUNT ON;",
+      get_clean_sql("../sql_scripts/simple.sql")
+    )
+  )
+})
+
+test_that("Rejects non-boolean values for additional_settings", {
+  expect_error(
+    get_clean_sql("../sql_scripts/simple.sql", additional_settings = "True"),
+    "additional_settings must be either TRUE or FALSE"
+  )
+  expect_error(
+    get_clean_sql("../sql_scripts/simple.sql", additional_settings = ""),
+    "additional_settings must be either TRUE or FALSE"
+  )
+})
+
+test_that("Rejects file that don't have SQL extension", {
+  expect_error(
+    get_clean_sql("../spelling.R"),
+    ""
+  )
+})
diff --git a/vignettes/.gitignore b/vignettes/.gitignore
@@ -0,0 +1,2 @@
+*.html
+*.R
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		-- Simple SQL script to get all data from my database table

		SELECT * FROM [my_database_table]