0
\$\begingroup\$

Problem

A business process requires me to read two different text files (raw and processed) into R as data.frames, with each column being read as a character type. Both files contain the same information, though the numeric columns are formatted slightly differently.

I need to convert the column types of both data.frames to the appropriate class, using special formatting codes. Both file types (raw/processed) use the same formatting codes for each column. I've created a function that accepts a data.frame, a character vector of formatting codes, and whether the data is raw or processed. This function then returns a data.frame with the proper columns.

Formatting Codes

EDIT: I don't have control over the formatting codes. For this bit of code the numbers in parentheses are not used. e.g., the "09" in "C(09)"

Formatting codes for each column follow 3 distinct patterns (of which there should be no overlap):

  • "C(05)": Represents a character with length 5. Other examples include "C(13)", "C(01)". The two digit number in parentheses will be between 01 and 99.
  • "N(09)": Represents integer with 9 numbers. Other examples include "N(13)", "N(01)". The two digit number in parentheses will be between 01 and 99.
  • "NNVNN": Represents numeric column with 4 numbers and an implied decimal between number 2 and 3 (e.g. 22.45). The "V" represents where the decimal should be.

Difference between raw and processed files

The main difference is how floating numbers are represented. In the raw file, there is only an implied decimal point whereas in the processed file the decimal point is explicit. The formatting code for floating numbers (e.g., "NNVNN") explains where the decimal should be. (The "V" is where the decimal place goes)

Goal for Review

I am open to any feedback at all. My primary concerns lie with the implementation of the which_regex function and the fmt_func function. It feels like there should be a better way to implement both in order to eventually scale the types of "files" (e.g., raw/processed). While I was hoping to have fmt_func vectorised, I was unable to do so and maintain a general readability.

This code was modified from the actual code I wrote, because I use a naming convention more specific to the business case in the actual code.

Data for Testing

# Source Data
raw <- data.frame(
    a = c("12345", "00123", "10000"),
    b = c("12345", "00123", "10000"),
    c = c("12345", "00123", "10000"),
    d = c("12345", "00123", "10000"),
    stringsAsFactors = FALSE
)

# Processed Data
processed <- data.frame(
    a = c("12345", "00123", "10000"),
    b = c("12345", "123", "10000"),
    c = c("12345", "1.23", "100."),
    d = c("12.345", ".123", "10."),
    stringsAsFactors = FALSE
)

# Format Specification Codes
# Each element maps to the columns in data above.
fmts <- c("C(05)", "N(05)", "NNNVNN", "NNVNNN")

Code Developed to Solve Problem

# Goal: Function that formats columns of a data.frame to desired specifications.
# Specifications are indicated by formatting codes.
fmt_data <- function(data, fmts, raw=TRUE){
    f <- purrr::map(fmts, fmt_func, raw=raw)
    
    purrr::map2_df(data, f, ~ .y(.x))
}


# Given a string `x`, what's the index of the supplied regex pattern that matches it.
which_regex <- function(x, patterns){
    matches <- unlist(lapply(patterns, grepl, x = x))
    match_matrix <- matrix(matches, ncol=length(patterns))
    indices <- which(match_matrix, arr.ind = TRUE)[,2]
    as.integer(indices)
}

# Chooses the column formatting function based on the format code
# I believe this function is technically a "function factory"?
fmt_func <- function(fmt, raw = TRUE){
    # These are regex patterns that match to the three types of formatting codes
    patterns <- c("^(C)\\([0-9]{2}\\)$", "^(N)\\([0-9]{2}\\)$",  "^N+V+N*$")
    
    # which_regex is defined above
    ind <- which_regex(fmt, patterns)
    
    # If it's a 'raw' data.frame, we'll need to specify where the decimal place goes,
    # so choose option 4
    opt <- ifelse(ind==3 & raw, 4, ind)
    
    f <- switch(opt,
                as.character,
                as.integer,
                as.numeric,
                function(x){
                    force(fmt)
                    as.numeric(x) / (10 ^ nchar(unlist(strsplit(fmt, "V"))[2]))
                }
    )
    
    f
}

Example Using Testing Data Above

fmt_data(raw, fmts, raw=TRUE)

fmt_data(processed, fmts, raw=FALSE)
```
\$\endgroup\$
6
  • 1
    \$\begingroup\$ Not sure if I understand all of it, but it looks a bit overcomplicated and not sure if you are happy with your final outcome. I noticed that actually for C(xx) and N(xx) nothing is done with the length (xx) part. but just does as.character or as.numeric. I had expected if in raw we had 123456 a C(4) or a C(99) would either cut off or pad it for example. Also your other ones just ignore all before the V and just set the dot at the position of V (starting from end). as on 123456 a NNVNNN gives me a NNNVNNN in practice. If that does not matter I think it can be far less complicated. \$\endgroup\$ Feb 16, 2022 at 12:59
  • \$\begingroup\$ Thanks for the feedback. For the scope of this bit of code, you're right. The numbers within the parentheses are not currently used. They are used in the same program (this function is one part of a larger package) to help read in data from a fixed width format. Unfortunately, the layout of the codes are not a feature I can control. \$\endgroup\$ Feb 16, 2022 at 15:28
  • \$\begingroup\$ If it seems overcomplicated, I would love to know where you think the over complication is and why. I'd love to be able to simplify it. I should probably note that there are many different data files (each with their own formats) that this function is designed to work on. \$\endgroup\$ Feb 16, 2022 at 15:31
  • \$\begingroup\$ It is just a gut feeling it is overcomplicated, but my feeling can be wrong if I did not understand you well enough. It is not per se the layout of the code but to me it seemed that the numbers within the () are not used, so C(nomatterthenumber) just means as.character of the value. Is that correct? If so you need to detect "C(", "N(" and a V position counted from right to left. to either make it a character, numeric or add the dot at the position of V \$\endgroup\$ Feb 16, 2022 at 15:46
  • 1
    \$\begingroup\$ Thanks for the extra confirmation on those specs. I think not much was wrong with your code, but as you mentioned I skipped the whole "which_regex" part. I hope you consider it useful and provided my solution worked out as an answer. \$\endgroup\$ Feb 17, 2022 at 14:27

1 Answer 1

1
\$\begingroup\$

Edit I am new to Code review and yes have provided answers on StackOverflow. As explained by @pacmaninbw my answer lacks clarity for a review so let me try to motivate my approach.

which_regex function is not needed as the switch to detect can be done on your sanitized expressions. The first switch case can easily be "C" and there is no need to translate that to 1. One could even argue using the letters makes it more readable and we save the need for that function.

The input fmts is more detailed than what the function actually does so can easily be translated up front. Like C(05) and C(99) do the same thing, same for N() and NNNVNNN the N's prior to V are always ignored. We can simply translate those to C, N, VN* strings. We then can use the switch based on C, N and use the default case to take the nchar-1 to get the position of the dots.

Perhaps switching to data.table could be considered personal preference so that can surely be argued as an improvement of code. But all above this to skip out the which_regex function is I think still a valid improvement you dan do while keeping your purr approach.

original post

After some thoughts and reading your comments, I think this could be a way to go.

function

fmt_data <- function(data, fmts, raw = TRUE) {
  fmts_translated <- gsub("^(C|N)\\(.+|^N+(V+N*$)", "\\1\\2", fmts) # could also be outside the function I think
  cols <- names(data)
  out <- data[, lapply(seq_along(names(.SD)), function(i) {
      switch(fmts_translated[[i]],
        "C" = as.character(.SD[[i]]),
        "N" = as.integer(.SD[[i]]),
        if(raw) as.numeric(.SD[[i]]) / (10 ^ nchar(fmts_translated[i])-1) else (as.numeric(.SD[[i]]))
      )
  }), .SDcols = cols]
  setnames(out, new = cols)
  return(out)
}

edit I think that fmts_translated <- gsub("^(C|N)\\(.+|^N+(V+N*$)", "\\1\\2", fmts) should be outside the function actually as it is depending on your fmts and if you happen to run your function many times on for example many tables there is no need to translate fmts over again for every function call if fmts remains the same.

usage and output note that I use data.table here

library(data.table)

# make it a data.table
setDT(raw)
setDT(processed)

fmt_data(raw, fmts, raw = TRUE)

#        a     b      c      d
# 1: 12345 12345 123.45 12.345
# 2: 00123   123   1.23  0.123
# 3: 10000 10000 100.00 10.000

# Classes ‘data.table’ and 'data.frame':    3 obs. of  4 variables:
#  $ a: chr  "12345" "00123" "10000"
#  $ b: int  12345 123 10000
#  $ c: num  123.45 1.23 100
#  $ d: num  12.345 0.123 10
#  - attr(*, ".internal.selfref")=<externalptr> 

fmt_data(processed, fmts, raw = FALSE)

#        a     b        c      d
# 1: 12345 12345 12345.00 12.345
# 2: 00123   123     1.23  0.123
# 3: 10000 10000   100.00 10.000

# Classes ‘data.table’ and 'data.frame':    3 obs. of  4 variables:
#  $ a: chr  "12345" "00123" "10000"
#  $ b: int  12345 123 10000
#  $ c: num  12345 1.23 100
#  $ d: num  12.345 0.123 10
#  - attr(*, ".internal.selfref")=<externalptr>

data

raw <- data.frame(
    a = c("12345", "00123", "10000"),
    b = c("12345", "00123", "10000"),
    c = c("12345", "00123", "10000"),
    d = c("12345", "00123", "10000"),
    stringsAsFactors = FALSE
)

# Processed Data
processed <- data.frame(
    a = c("12345", "00123", "10000"),
    b = c("12345", "123", "10000"),
    c = c("12345", "1.23", "100."),
    d = c("12.345", ".123", "10."),
    stringsAsFactors = FALSE
)

# Format Specification Codes
# Each element maps to the columns in data above.
fmts <- c("C(05)", "N(05)", "NNNVNN", "NNVNNN")
\$\endgroup\$
5
  • 1
    \$\begingroup\$ This would be an excellent answer on stack overflow, here on Code Review it is missing any observations about the original code which is the point of the code review site. Alternate code only answers are considered poor answers. The point of code review is to help the original poster by making insightful observations about there code. If you add an explanation about why your code is better this would be a better answer. \$\endgroup\$
    – pacmaninbw
    Feb 17, 2022 at 15:51
  • \$\begingroup\$ Hi @Merjin van Tilborg. Thank you for spending the time to consider an alternative to my code - I appreciate it. I like that you extracted the unique part of the formatting codes, and use them in the switch statement. In general, I think our code is similar on a fundamental level. Instead of creating a vector of functions to iterate over, you just apply the functions at the time of iteration. I'd love to know how/why this might be better? You also remove the purrr dependency, which I appreciate, but introduce the data.table dependency (which I'm less familiar with). \$\endgroup\$ Feb 17, 2022 at 16:24
  • \$\begingroup\$ I think there may be a bug in your code, though, if I were to have a column that required the following format: "NVN". I think this would get cast as an integer, instead of a numeric. \$\endgroup\$ Feb 17, 2022 at 16:25
  • \$\begingroup\$ To prevent tha issue with NVN change the regex to keep VN* and the length will be -1 \$\endgroup\$ Feb 18, 2022 at 8:37
  • 1
    \$\begingroup\$ I added some motivation as well as a fix on the NVN cases \$\endgroup\$ Feb 18, 2022 at 9:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.