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

Clean obsolete files in R, tests or vignettes #24

Closed
4 tasks done
statnmap opened this issue Jul 9, 2021 · 4 comments
Closed
4 tasks done

Clean obsolete files in R, tests or vignettes #24

statnmap opened this issue Jul 9, 2021 · 4 comments
Assignees
Labels

Comments

@statnmap
Copy link
Member

statnmap commented Jul 9, 2021

Validation

During inflate_all():

  • I would like a function at the scale of my package that tells me if a file in R, tests or vignettes is not legitimate : not coming from a flat file, or specifically registered by the user
  • I would like this same function to ask me if it can delete this non-legitimate files. It asks with a question or use the parameter clean = TRUE to delete without question.
  • I would like a function that detects and informs, during the inflate of a specific flat file, if I am about to make obsolete some R, test or vignettes files.
  • If I change the name of the main function of a function chunk, a new R, tests files will be created. I would like the R and tests files of the older name to be deleted when I inflate my flat file, whether I am asked or directly with clean = TRUE

Tech

Development will mainly take place in : "dev/flat_clean_fusen_files.Rmd"

Level of the package

  • Modify check_not_registered_files() to ask if: you want to delete all or you want to register all (hence, run register_all_to_config()) or register only a part of it ?
  • Create a specific function named delete_all_not_registered() for the delete part
  • If the user want to "register only a part of it", for now, the message would be : modify the csv file "config_not_registered.csv" and then we need to tell them to run delete_all_not_registered() after that.
    • Maybe we can make this interactive in a way/

Level of the inflate of one flat file

  • Modify the df_to_config(flat_file_path = relative_flat_file) when used during inflate() so that the clean = TRUE parameter also asks to clean R, tests or vignettes
    • For now, it only delete the part of the "fusen_config.yml" that listed previous version and update it. It does not deal with the files in R, tests or vignettes created before.
@MargotBr
Copy link

Hello @statnmap,

I don't know if this helps, but here is the code I produced to identify the "obsolete" functions.

It compares the functions present in the "flat" files (using the names of the chunks r function-...) and the functions present in the R/ directory.

I excluded the functions from the package that were not built with flat files (notably functions related to the shiny app, and utils functions).

identify_fct_in_flat <- function(name_flat_rmd) {

  flat_in_line <- readLines(file.path("dev", name_flat_rmd))

  names_fct_in_flat <- flat_in_line[which(stringr::str_detect(flat_in_line, "```\\{r function-"))] %>%
    stringr::str_remove_all("```\\{r function-") %>%
    stringr::str_remove_all("\\}")

  return(names_fct_in_flat)

}

fct_in_flat_files <- list.files("dev")[list.files("dev") %>% stringr::str_detect("^flat")] %>%
  purrr::map(identify_fct_in_flat) %>%
  unlist() %>%
  stringr::str_c(".R")

fct_in_r_folder <- list.files("R")

obsolete_fcts <- fct_in_r_folder[!(fct_in_r_folder %in% fct_in_flat_files)]
obsolete_fcts <- obsolete_fcts[!grepl("app|mod_|zzz|listofcodes-package|utils-pipe|welcome_banner", obsolete_fcts)]
obsolete_fcts

@ColinFay
Copy link
Member

Hey,

Quick thoughts on this code :

  • you can create function that do no match the function- chunk name
    ```{r function-https, file.name = "https"}
#' HTTP req
#'
#' @importFrom cli cli_alert_success cli_alert_danger
#' @importFrom httr GET POST add_headers status_code content
#'
#' @return Un objet response de {httr}, lisible avec httr::content()
#' @noRd
#' @rdname HTTP
health_check <- function(
  url
) {
  # [...]
}
    ```
  • you can have several functions in the same chunk
    ```{r function-https, file.name = "https"}
#' HTTP req
#'
#' @importFrom cli cli_alert_success cli_alert_danger
#' @importFrom httr GET POST add_headers status_code content
#'
#' @return Un objet response de {httr}, lisible avec httr::content()
#' @noRd
#' @rdname HTTP
health_check <- function(
  url
) {
  # [...]
}

#' @noRd
#' @rdname HTTP
delete_ <- function(url, expected_status_code = 200) {
  # [...]
}
    ```

@statnmap statnmap added this to the v0.5 milestone Jul 29, 2022
statnmap added a commit that referenced this issue Jul 30, 2022
Tags: feat, doc, test

Ongoing feature creation.

- Retrieve file path created during inflate
- Start to store in a yaml file

Issue #24
statnmap added a commit that referenced this issue Jul 31, 2022
Tags: feat, doc, test

Why?

- To create a config file for already existing {fusen}, we need not to detect files that should not be deleted

What?

WIP:

- List all files and try to guess origin
- Show the user all files not registered and allow them to manually clean

issue #24
statnmap added a commit that referenced this issue Aug 1, 2022
Tags: feat, test, doc

Why?

- Need to migrate package to the new config file management

What?

- Allow `df_to_config()` to run on the output of `check_not_registered_files()`

Issue #24
statnmap added a commit that referenced this issue Aug 9, 2022
tags: feat, test, doc

Why?

- Users would want to use the new config file in the new fusen version, without having to write it themselves if coming from older version or classical package

What?

- Add `register_all_to_config()`

Issue #24
statnmap added a commit that referenced this issue Aug 9, 2022
tags: doc

Why?

- Function is done, it can be inflated in the package

What?

- Just inflate

issue #24
statnmap added a commit that referenced this issue Sep 17, 2022
Tags: feat, doc, test

Ongoing feature creation.

- Retrieve file path created during inflate
- Start to store in a yaml file

Issue #24
statnmap added a commit that referenced this issue Sep 17, 2022
Tags: feat, doc, test

Why?

- To create a config file for already existing {fusen}, we need not to detect files that should not be deleted

What?

WIP:

- List all files and try to guess origin
- Show the user all files not registered and allow them to manually clean

issue #24
statnmap added a commit that referenced this issue Sep 17, 2022
Tags: feat, test, doc

Why?

- Need to migrate package to the new config file management

What?

- Allow `df_to_config()` to run on the output of `check_not_registered_files()`

Issue #24
statnmap added a commit that referenced this issue Sep 17, 2022
tags: feat, test, doc

Why?

- Users would want to use the new config file in the new fusen version, without having to write it themselves if coming from older version or classical package

What?

- Add `register_all_to_config()`

Issue #24
statnmap added a commit that referenced this issue Sep 17, 2022
tags: doc

Why?

- Function is done, it can be inflated in the package

What?

- Just inflate

issue #24
statnmap added a commit that referenced this issue Sep 17, 2022
tags: fix, doc, test

Why?

- All examples need to run properly

What?

- UPdate flat file for clean_fusen_files
- Update documentation to explain how it will work
- fix tests and examples

Issue #24
statnmap added a commit that referenced this issue Oct 15, 2022
Tags: feat, doc, test

Ongoing feature creation.

- Retrieve file path created during inflate
- Start to store in a yaml file

Issue #24
statnmap added a commit that referenced this issue Oct 15, 2022
Tags: feat, doc, test

Why?

- To create a config file for already existing {fusen}, we need not to detect files that should not be deleted

What?

WIP:

- List all files and try to guess origin
- Show the user all files not registered and allow them to manually clean

issue #24
statnmap added a commit that referenced this issue Oct 15, 2022
Tags: feat, test, doc

Why?

- Need to migrate package to the new config file management

What?

- Allow `df_to_config()` to run on the output of `check_not_registered_files()`

Issue #24
statnmap added a commit that referenced this issue Oct 15, 2022
tags: feat, test, doc

Why?

- Users would want to use the new config file in the new fusen version, without having to write it themselves if coming from older version or classical package

What?

- Add `register_all_to_config()`

Issue #24
statnmap added a commit that referenced this issue Oct 15, 2022
tags: doc

Why?

- Function is done, it can be inflated in the package

What?

- Just inflate

issue #24
statnmap added a commit that referenced this issue Oct 15, 2022
tags: fix, doc, test

Why?

- All examples need to run properly

What?

- UPdate flat file for clean_fusen_files
- Update documentation to explain how it will work
- fix tests and examples

Issue #24
statnmap added a commit that referenced this issue Oct 15, 2022
tags: fix, test, doc

Why?

- `data.frame()` and `read.csv()` read string as factors in old R versions, preventing them to be used as file path

What?

- Added stringAsFactors = FALSE everywhere needed

issue #24
@statnmap statnmap mentioned this issue Oct 15, 2022
6 tasks
statnmap added a commit that referenced this issue Oct 16, 2022
tags: feat, doc, test

Why?

- More info needs to be stored in the config_fusen.yaml file.

What?

- Allow test to be ok with extra information
- TODO: define 'df_to_config()' to only bother about path, R, tests, vignette

issue #24
@statnmap
Copy link
Member Author

A function to detect if there are duplicate names in "R/".
This requires to extract parse_fun_vec from parse_fun() that is already in {fusen}

parse_fun_vec <- function(code) {
  # code <- unlist(rmd_node_code(x[["ast"]]))
  regex_isfunction <- paste("function(\\s*)\\(", "R6Class(\\s*)\\(", 
                            sep = "|")
  regex_extract_fun_name <- paste("[\\w[.]]*(?=(\\s*)(<-|=)(\\s*)function)", 
                                  "[\\w[.]]*(?=(\\s*)(<-|=)(\\s*)R6Class)", "[\\w[.]]*(?=(\\s*)(<-|=)(\\s*)R6::R6Class)", 
                                  sep = "|")
  fun_name <- stringi::stri_extract_first_regex(code[grep(regex_isfunction, 
                                                          code)], regex_extract_fun_name) %>% gsub(" ", "", .)
  code <- gsub(pattern = "#'\\s*@", "#' @", code)
  first_function_start <- grep(regex_isfunction, code)[1]
  all_hastags <- grep("^#'", code)
  if (length(all_hastags) != 0) {
    last_hastags_above_first_fun <- max(all_hastags[all_hastags < 
                                                      first_function_start])
  }
  else {
    last_hastags_above_first_fun <- NA
  }
  if (!any(grepl("@export|@noRd", code))) {
    if (!is.na(last_hastags_above_first_fun)) {
      code <- c(code[1:last_hastags_above_first_fun], "#' @noRd", 
                code[(last_hastags_above_first_fun + 1):length(code)])
    }
    else if (all(grepl("^\\s*$", code))) {
      code <- character(0)
    }
    else if (!is.na(first_function_start)) {
      code <- c("#' @noRd", code)
    }
  }
  all_arobase <- grep("^#'\\s*@|function(\\s*)\\(", code)
  example_pos_start <- grep("^#'\\s*@example", code)[1]
  example_pos_end <- all_arobase[all_arobase > example_pos_start][1] - 
    1
  example_pos_end <- ifelse(is.na(example_pos_end), grep("function(\\s*)\\(", 
                                                         code) - 1, example_pos_end)
  tag_filename <- gsub("^#'\\s*@filename\\s*", "", code[grep("^#'\\s*@filename", 
                                                             code)])
  tag_rdname <- gsub("^#'\\s*@rdname\\s*", "", code[grep("^#'\\s*@rdname", 
                                                         code)])
  rox_filename <- c(tag_filename, tag_rdname)[1]
  code[grep("^#'\\s*@filename", code)] <- "#'"
  tibble::tibble(fun_name = fun_name[1], code = list(code), 
                 example_pos_start = example_pos_start, example_pos_end = example_pos_end, 
                 rox_filename = rox_filename)
}


get_duplicate_functions <- function(path = "R") {
  all_r <- list.files(path, full.names = TRUE)
  all_funs <- lapply(all_r, function( one_r) {
    # one_r <- all_r[1]
    r_lines <- readLines(one_r)
    parse_fun_vec(r_lines)
  })
  res <- do.call("rbind", all_funs)
  res_clean <- res[!is.na(res[["fun_name"]]),]
  res_dups <- res_clean[["fun_name"]][duplicated(res_clean[["fun_name"]])]
  
  if (length(res_dups) != 0) {
    message('There are duplicated function names:', paste(res_dups, collapse = ", "))
  }
  list(
    all_funs = res_clean,
    duplicated = res_dups
  )
}
}

statnmap added a commit that referenced this issue Dec 20, 2022
tags: feat, doc

Why?

- Next {fusen} version will come with big changes, the version number will be an index to trigger (or not) new functions

What?

- Add {fusen} version number in the DESCRIPTION file

issue #24
statnmap added a commit that referenced this issue Dec 21, 2022
tags: fix, doc

Why?

Syntax with `Config/` does not fail on CRAN

issue #24
statnmap added a commit that referenced this issue Dec 21, 2022
Keep both version when conflicts

Issue #24

Merge branch '24-rebase2-clean-yourself' into 24-rebase3-clean-yourself

# Conflicts:
#	R/inflate.R
#	tests/testthat/test-inflate-part2.R
@statnmap statnmap mentioned this issue Dec 21, 2022
8 tasks
statnmap added a commit that referenced this issue Jan 3, 2023
tags: fix, feat, test, doc

Why?

- We may want to register some files after some manual modifications

issue #24
statnmap added a commit that referenced this issue Jan 3, 2023
tags: chore, feat, test, doc

Why?

- It is time to integrate the new feature to the mainstream.
- The sets a major step towards the cleaning functions to be coming soon (#24)

What?

- Bump version but keep it as a .9000 to avoid updating vignettes in the pkgdown file
- Change path in "do not edit by hand" for a relative path that do not start with "/"

issue #24
@statnmap statnmap changed the title Check if functions in R files are consistent with dev_history Clean obsolete files in R, tests or vignettes Feb 23, 2023
@statnmap statnmap self-assigned this Jun 13, 2023
statnmap added a commit that referenced this issue Jun 13, 2023
tags: feat, test

- Include this feature in the next release so that users will benefit from the cleaning tools in the next release
- Encourage to use `register_all_to_cnofig()` for the transition to this new version of {fusen}
- Requires doc for inflate_all()
- requires unit tests to check that this functionnality works correctly

issue #24
statnmap added a commit that referenced this issue Jun 14, 2023
tags: feat, doc, test

- check_unregistered_file during inflate_all()
- This can be avoided with `clean=FALSE`
- Update doc links between functions

issue #24
statnmap added a commit that referenced this issue Jun 14, 2023
tags: fix, test

issue #24
statnmap added a commit that referenced this issue Jun 14, 2023
tags: fix, test

- R <= 3.6 requires change default stringasfactors parameter

issue #24
@statnmap statnmap added v0.6 and removed v0.5 labels Aug 17, 2023
statnmap added a commit that referenced this issue Apr 30, 2024
Delete files that are no longer needed when detected by inflating

issue #24
@statnmap
Copy link
Member Author

statnmap commented May 2, 2024

Since 'fusen' registers files created during inflate in the config_fusen file, it knows what was the name of the created files in the previous inflate. Hence, we can ask to 'clean' these files directly.
By default, it asks when such a deleted filename occurs, but you can specifiy clean=TRUE in the inflate options.
I recommand to use inflate_all() too, to take care of functions that you moved from one flat to the other, so that you do not forget to inflate each file modified.

@statnmap statnmap closed this as completed May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants