Skip to content

Commit

Permalink
Streamline and tighten lintr checks (#220)
Browse files Browse the repository at this point in the history
* Replace lint.yaml by lint-changed-file.yaml

Copied from r-lib/actions with some extra bells and whistles copied from lint.yaml

- triggered on develop and can be triggered manually
- can be skipped with the [ci skip] instruction in the commit message

* Try to include all linters

* Fix some indents

Indents in function argument definition have not been changed because they are an explicit style choice

* Add fixed = TRUE to calls with fixed regexes

* Add leading zero to double

* Use toString() rather than paste(collapse = ", ")

* Use anyNA() rather than any(is.na())

* Remove unused variables

* Pass function directly to sapply instead of creating lambda

* Use dedicated lengths() function

* Remove unnecessary library() calls

* Use file.path() to construct file paths

* Replace unnecessary ifelse() by as.numeric()

* Remove unnecessary paste0() in warning() and stop()

* Remove unnecessary c()

* Use seq_len() rather than :

* Use sort() rather than order()

* Remove nested if

* Disable linters that generate too many false positive
or that flag explicit style choices that diverge from default

* Run devtools::document()

* Add NEWS items

* Increment version number

* Remove extra trailing whitespace
  • Loading branch information
Bisaloo committed Apr 11, 2023
1 parent 4c760f6 commit 8485137
Show file tree
Hide file tree
Showing 37 changed files with 236 additions and 205 deletions.
54 changes: 54 additions & 0 deletions .github/workflows/lint-changed-files.yaml
@@ -0,0 +1,54 @@
# Workflow derived from https://github.com/r-lib/actions/tree/v2/examples
# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help
on:
push:
branches:
- main
- master
- develop
pull_request:
branches:
- main
- master
- develop
workflow_dispatch:

name: lint-changed-files

jobs:
lint-changed-files:
runs-on: ubuntu-latest
if: "! contains(github.event.head_commit.message, '[ci skip]')"
env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
steps:
- uses: actions/checkout@v3

- uses: r-lib/actions/setup-r@v2

- uses: r-lib/actions/setup-r-dependencies@v2
with:
extra-packages: |
any::gh
any::lintr
any::purrr
needs: check

- name: Add lintr options
run: |
cat('\noptions(lintr.linter_file = ".lintr")\n', file = "~/.Rprofile", append = TRUE)
shell: Rscript {0}

- name: Install package
run: R CMD INSTALL .

- name: Extract and lint files changed by this PR
run: |
files <- gh::gh("GET https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files")
changed_files <- purrr::map_chr(files, "filename")
all_files <- list.files(recursive = TRUE)
exclusions_list <- as.list(setdiff(all_files, changed_files))
lintr::lint_package(exclusions = exclusions_list)
shell: Rscript {0}
env:
LINTR_ERROR_ON_LINT: true
36 changes: 0 additions & 36 deletions .github/workflows/lint.yaml

This file was deleted.

22 changes: 15 additions & 7 deletions .lintr
@@ -1,7 +1,15 @@
linters: linters_with_defaults(
line_length_linter = line_length_linter(80),
cyclocomp_linter = cyclocomp_linter(complexity_limit = 20L),
object_usage_linter = NULL)
exclusions: c(list.files(path = "tests/", recursive = T, full.names = T),
"vignettes/germany-age-stratified-nowcasting.Rmd.orig")
exclude: "# nolint"
linters: linters_with_tags(
tags = NULL, # include all linters
implicit_integer_linter = NULL,
extraction_operator_linter = NULL,
undesirable_function_linter = NULL,
function_argument_linter = NULL,
indentation_linter = NULL,
object_name_linter = NULL,
cyclocomp_linter(25L)
)
exclusions: c(
list.files("tests", recursive = TRUE, full.names = TRUE),
list.files("inst", recursive = TRUE, full.names = TRUE),
"vignettes/germany-age-stratified-nowcasting.Rmd.orig"
)
2 changes: 1 addition & 1 deletion DESCRIPTION
@@ -1,6 +1,6 @@
Package: epinowcast
Title: Flexible Hierarchical Nowcasting
Version: 0.2.0.7000
Version: 0.2.0.8000
Authors@R:
c(person(given = "Sam Abbott",
role = c("aut", "cre"),
Expand Down
4 changes: 3 additions & 1 deletion NEWS.md
@@ -1,4 +1,4 @@
# epinowcast 0.2.0.7000
# epinowcast 0.2.0.8000

This is release is in development. It is not yet ready for production use. If you notice problems please report them on the [issue tracker](https://github.com/epinowcast/epinowcast/issues).

Expand All @@ -9,6 +9,8 @@ This is release is in development. It is not yet ready for production use. If yo

## Package

- Added more non-default linters in `.lintr` configuration file. This file is used when `lintr::lint_package()` is run or in the new `lint-changed-files.yaml` GitHub Actions workflow. See #220 by @Bisaloo and reviewed by @pearsonca and @seeabs.
- Switched to the `lint-changed-files.yaml` GitHub Actions workflow instead of the regular `lint.yaml` to avoid annotations unrelated to the changes made in the PR. See #220 by @Bisaloo and reviewed by @pearsonca and @seeabs.
- Added tests for `summary.epinowcast()` and `plot.epinowcast()` methods. See #209 by @seabbs and reviewed by @pearsonca.
- Added tests for `enw_plot_obs()` where not otherwise covered by `plot.epinowcast()` tests. See #209 by @seabbs and reviewed by @pearsonca.
- Made the `.group` variable optional for all preprocessing functions using a new `add_group()` internal function. See #208 by @seabbs and reviewed by @pearsonca.
Expand Down
15 changes: 8 additions & 7 deletions R/check.R
Expand Up @@ -17,7 +17,7 @@ check_quantiles <- function(posterior, req_probs = c(0.5, 0.95, 0.2, 0.8)) {
if (sum(cols %in% paste0("q", req_probs * 100)) != length(req_probs)) {
stop(
"Following quantiles must be present (set with probs): ",
paste(req_probs, collapse = ", ")
toString(req_probs)
)
}
return(invisible(NULL))
Expand Down Expand Up @@ -88,16 +88,16 @@ check_group <- function(obs) {
#' @return NULL
#'
#' @family check
check_by <- function(obs, by = c()) {
check_by <- function(obs, by = NULL) {
if (length(by) > 0) {
if (!is.character(by)) {
stop("`by` must be a character vector")
}
if (!all(by %in% colnames(obs))) {
stop(
"`by` must be a subset of the columns in `obs`. \n",
paste0(paste(by[!(by %in% colnames(obs))], collapse = ", "),
" are not present in `obs`")
toString(by[!(by %in% colnames(obs))]),
" are not present in `obs`"
)
}
}
Expand Down Expand Up @@ -151,15 +151,16 @@ check_modules_compatible <- function(modules) {
modules[[4]]$data$model_miss &&
!modules[[6]]$data$likelihood_aggregation
) {
warning(paste0(
warning(
"Incompatible model specification: A missingness model has ",
"been specified but likelihood aggregation is specified as ",
"by snapshot. Switching to likelihood aggregation by group.",
" This has no effect on the nowcast but limits the ",
"number of threads per chain to the number of groups. To ",
"silence this warning, set the `likelihood_aggregation` ",
"argument in `enw_fit_opts` to 'groups'."
), immediate. = TRUE)
"argument in `enw_fit_opts` to 'groups'.",
immediate. = TRUE
)
}
return(invisible(NULL))
}
4 changes: 2 additions & 2 deletions R/epinowcast.R
Expand Up @@ -115,9 +115,9 @@ epinowcast <- function(data,
),
expectation = epinowcast::enw_expectation(
r = ~ 0 + (1 | day:.group),
generation_time = c(1),
generation_time = 1,
observation = ~1,
latent_reporting_delay = c(1),
latent_reporting_delay = 1,
data = data
),
missing = epinowcast::enw_missing(
Expand Down

0 comments on commit 8485137

Please sign in to comment.