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

read_documentation is ambiguous #3

Open
1 task done
Tomeriko96 opened this issue May 3, 2024 · 5 comments
Open
1 task done

read_documentation is ambiguous #3

Tomeriko96 opened this issue May 3, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@Tomeriko96
Copy link
Member

Tomeriko96 commented May 3, 2024

The name read_documentation() is, in hindsight, quite ambiguous.

  • Create a new function called read_import_definitions that follows the same pattern as read_documentation.
@Tomeriko96
Copy link
Member Author

We could resort to the following function instead:

#' read_import_definitions
#'
#' A function that reads an import definitions file.
#'
#' @param filename The filename of the import definitions file.
#' @param metadata_location Location of the import definitions files,
#' if NULL system variables will be used.
#' @param ... Additional parameters to be passed to the file reading function.
#'
#' @return A data frame containing the import definitions.
#' @export
read_import_definitions <- function(filename = "", metadata_location = NULL, ...) {
  if (is.null(metadata_location)) {
    if (!Sys.getenv("METADATA_IMPORT_DEFINITIONS_DIR") == "") {
      metadata_location <- Sys.getenv("METADATA_IMPORT_DEFINITIONS_DIR")
    } else {
      stop("System variable for metadata_location is missing.")
    }
  }

  file_path <- file.path(metadata_location, filename)

  # Determine the file reading function based on the file extension
  file_extension <- tools::file_ext(filename)
  if (file_extension == "csv") {
    return(utils::read.csv(file_path, stringsAsFactors = FALSE, ...))
  } else if (file_extension == "txt" || file_extension == "dat") {
    # Assume tab-separated file
    return(utils::read.delim(file_path, stringsAsFactors = FALSE, ...))
  } else {
    stop("Unsupported file format. Only CSV, TXT, and DAT files are supported.")
  }
}

  • Naming: Clearly uses import definitions files
  • System variable: A clear name to differentiate from other metadata/documentation

@Tomeriko96
Copy link
Member Author

@CorneeldH Do you agree with this function?

@CorneeldH
Copy link

Yes, agreed 👍 Do you also want to supersede read_documenation, with read_data_dictionary or read_dictionary or something else less generic than read_documenation?

https://lifecycle.r-lib.org/articles/stages.html

@Tomeriko96
Copy link
Member Author

Tomeriko96 commented May 6, 2024

That sounds reasonable enough:

We will create (pending the naming)

  • read_import_definitions
  • read_data_dictionary

As you mention, the goal then is to supersede read_documentation, in favor of more specific functions. These have their own system variables to differentiate between them.

@Tomeriko96
Copy link
Member Author

read_data_dictionary has been added to the dev version:

#' read_data_dictionary
#'
#' A function that reads a data dictionary file.
#'
#' @param filename The filename of the data dictionary file.
#' @param metadata_location Location of the data dictionary files,
#' if NULL system variables will be used.
#' @param ... Additional parameters to be passed to the file reading function.
#'
#' @return A data frame containing the data dictionary.
#' @export
read_data_dictionary <- function(filename = "", metadata_location = NULL, ...) {
  if (is.null(metadata_location)) {
    if (!Sys.getenv("METADATA_DICTIONARY_DIR") == "") {
      metadata_location <- Sys.getenv("METADATA_DICTIONARY_DIR")
    } else {
      stop("System variable for metadata_location is missing.")
    }
  }
  
  file_path <- file.path(metadata_location, filename)
  
  # Determine the file reading function based on the file extension
  file_extension <- tools::file_ext(filename)
  if (file_extension == "csv") {
    return(utils::read.csv(file_path, stringsAsFactors = FALSE, ...))
  } else if (file_extension == "txt" || file_extension == "dat") {
    # Assume tab-separated file
    return(utils::read.delim(file_path, stringsAsFactors = FALSE, ...))
  } else {
    stop("Unsupported file format. Only CSV, TXT, and DAT files are supported.")
  }
}

It relies on system variable: METADATA_DICTIONARY_DIR. We can always choose a different name for it afterwards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants