rfm_segment() overwrites existing assignment in current implementation

leungi commented 4 years ago

Problem

Expectation:

running rfm_segment() on the example vignette, customers with rfm_score of 555 should be assigned Champions tag, per input thresholds, such as the customer below.

Reality:

however, it's assigned the next level down - Loyal Customers

segments[5, ] %>% select(customer_id:rfm_score)
#> # A tibble: 1 x 3
#>   customer_id    segment         rfm_score
#>   <chr>          <chr>               <dbl>
#> 1 Agness O'Keefe Loyal Customers       555

Suggestion

In conditional logic for assigning segments, add an extra condition such as:

  for (i in seq_len(n_segments)) {
    rfm_score_table$segment[(
      (rfm_score_table$recency_score %>% dplyr::between(recency_lower[i], recency_upper[i])) &
        (rfm_score_table$frequency_score %>% dplyr::between(frequency_lower[i], frequency_upper[i])) &
        (rfm_score_table$monetary_score %>% dplyr::between(monetary_lower[i], monetary_upper[i]))) &
         # new condition to avoid overwriting existing assignment
         !rfm_score_table$segment %in% segment_names] <- segment_names[i]
  }

Reprex

library(rfm)

analysis_date <- lubridate::as_date("2006-12-31", tz = "UTC")
rfm_result <- rfm_table_order(rfm_data_orders, customer_id, order_date, revenue, analysis_date)
rfm_result

segment_names <- c(
  "Champions", "Loyal Customers", "Potential Loyalist",
  "New Customers", "Promising", "Need Attention", "About To Sleep",
  "At Risk", "Can't Lose Them", "Lost"
)
recency_lower <- c(4, 2, 3, 4, 3, 2, 2, 1, 1, 1)
recency_upper <- c(5, 5, 5, 5, 4, 3, 3, 2, 1, 2)
frequency_lower <- c(4, 3, 1, 1, 1, 2, 1, 2, 4, 1)
frequency_upper <- c(5, 5, 3, 1, 1, 3, 2, 5, 5, 2)
monetary_lower <- c(4, 3, 1, 1, 1, 2, 1, 2, 4, 1)
monetary_upper <- c(5, 5, 3, 1, 1, 3, 2, 5, 5, 2)
segments <-
  rfm_segment(
    rfm_result,
    segment_names,
    recency_lower,
    recency_upper,
    frequency_lower,
    frequency_upper,
    monetary_lower,
    monetary_upper
  )

leungi commented 4 years ago

If this is the intended logic for rfm_segment(), I can do a PR 😃

aravindhebbali commented 4 years ago

HI @leungi Thanks for bringing this to our attention. Will take a look at it and get back to you.

aravindhebbali commented 4 years ago

Hi @leungi So whenever the intervals specified for one segment (Champions in this case) is a subset of another segment (Loyal Customers), rfm_segment() overwrites the existing assignment. Please do a pull request to the develop branch and we will be happy to merge 😃

rsquaredacademy / rfm