Lubridate: quarter() function bug

[awfrankwils]

First, I would like to thank the authors @hadley @jiho @mmparker @zeehio @garrettgman for this package, and in particular the development of the quarter() function which does a great job of reducing the amount of code needed to generate FY and Quarter values. I would like to point out that there currently appears to be a bug in the way the quarter() function appends a fiscal year with_year.

Original example
x <- ymd(c("2012-03-26", "2012-05-04", "2012-09-23", "2012-12-31"))
quarter(x, with_year = TRUE, fiscal_start = 11)
[1] 2012.2 2012.3 2012.4 2013.1
This works fine with the december 2012 date reported as fiscal year 2013 Q1.

However if I move the fiscal start to April, we don't see similar treatment for fiscal year of the first date -- which should be 2011 Q4 based on this formatting.
quarter(x, with_year = TRUE, fiscal_start = 4)
[1]2012.42012.1 2012.2 2012.3

[borgmaan]

I believe there is still a bug in the function for fiscal_start of 7, 8, or 9. Here's what I'm seeing with the latest version installed from GitHub:

> x
[1] "2012-03-26" "2012-05-04" "2012-09-23" "2012-12-31"
> quarter(x, with_year = TRUE, fiscal_start = 7)
[1] 2012.3 2012.4 2012.1 2012.2
> quarter(x, with_year = TRUE, fiscal_start = 8)
[1] 2012.3 2012.4 2012.1 2012.2
> quarter(x, with_year = TRUE, fiscal_start = 9)
[1] 2012.3 2012.3 2012.1 2012.2

I'm running:

Session info ------------------------------------------------------------------------------------------------------------------------
 setting  value                       
 version  R version 3.4.4 (2018-03-15)
 system   x86_64, darwin15.6.0        
 ui       RStudio (1.1.453)           
 language (EN)                        
 collate  en_US.UTF-8                 
 tz       America/New_York            
 date     2018-07-23                  

With:

Packages ----------------------------------------------------------------------------------------------------------------------------
 package      * version  date       source                              
...
lubridate    * 1.7.4    2018-07-23 Github (tidyverse/lubridate@f7a7c27)
...

[vspinu]

Indeed. Looks like the logic of quarter is plain bogus. Will have to carefully dig into it.

[borgmaan]

It's probably worth documenting the expected behavior of the function. FWIW, Wikipedia says:

The fiscal year is usually denoted by the year in which it ends, so United States federal government spending incurred on 14 November 2018 would belong to fiscal year 2019, operating on a fiscal calendar of October–September.

That seems to be the expected behavior of most of the tests and what is described in the bug report above. However, one of the tests from the commit closing this issue seems to violate that expectation.

I don't know what is right, but following that convention, I believe the test should be updated to:

x <- ymd(c("2012-03-26", "2012-05-04", "2012-09-23", "2012-03-01", "2012-12-31"))
expect_equal(quarter(x, with_year = TRUE, fiscal_start = 4),
             c(2012.4, 2013.1, 2013.2, 2012.4, 2013.3))

The following function passes all other tests (and the modified one above):

quarter <- function(x, with_year = FALSE, fiscal_start = 1) {
  fs <- fiscal_start - 1
  shifted <- seq(fs, 11 + fs) %% 12 + 1
  m <- month(x)
  quarters <- rep(1:4, each = 3)
  s <- match(m, shifted)
  q <- quarters[s]
  if (with_year) {
    uq <- quarters[m]
    inc_year <- (m >= fiscal_start) * (fiscal_start != 1)
    year(x) + inc_year + q/10
  }
  else q
}

Happy to submit a PR if this is the expected behavior and solves the issue. Here's the full diff for reference:

diff --git a/R/accessors-quarter.r b/R/accessors-quarter.r
index d299b64..c4bcf55 100644
--- a/R/accessors-quarter.r
+++ b/R/accessors-quarter.r
@@ -29,9 +29,8 @@ quarter <- function(x, with_year = FALSE, fiscal_start = 1) {
   q <- quarters[s]
   if (with_year) {
     uq <- quarters[m]
-    inc_year <- q == 1 & uq == 4
-    dec_year <- q == 4 & uq == 1
-    year(x) + inc_year - dec_year + q/10
+    inc_year <- (m >= fiscal_start) * (fiscal_start != 1)
+    year(x) + inc_year + q/10
   }
   else q
 }
diff --git a/tests/testthat/test-accessors.R b/tests/testthat/test-accessors.R
index 8685192..f7fe0fa 100644
--- a/tests/testthat/test-accessors.R
+++ b/tests/testthat/test-accessors.R
@@ -239,7 +239,7 @@ test_that("quarters accessor extracts correct quarter", {

   x <- ymd(c("2012-03-26", "2012-05-04", "2012-09-23", "2012-03-01", "2012-12-31"))
   expect_equal(quarter(x, with_year = TRUE, fiscal_start = 4),
-               c(2011.4, 2012.1, 2012.2, 2011.4, 2012.3))
+               c(2012.4, 2013.1, 2013.2, 2012.4, 2013.3))
   expect_equal(quarter(x, with_year = TRUE, fiscal_start = 11),
                c(2012.2, 2012.3, 2012.4, 2012.2, 2013.1))
 })

[rlh1994]

@borgmaan Was there a resolution to this?

[mielniczuk]

It appears the bug still exists for fiscal years starting in month 4 (or 3 or 2). I would expect that a FY starting April would show 2019-01-10 as FY 2018 Qtr 4 or 2018.4 result for the quarter() function when with_year=TRUE. Currently
quarter('2019-01-10', with_year = TRUE, fiscal_start = 4) produces 2019.4

[yhamade]

While this doesn't conform to the "Fiscal Quarters" definition covered by @borgmaan, it does address the incorrect year for fiscal_start=4 outlined by @mielniczuk to produce an output of 2018.4 (Fiscal Years that START in April and end in Mach of the following year).

quarter <- function(x, with_year = FALSE, fiscal_start = 1) {
   fs <- fiscal_start - 1
   shifted <- seq(fs, 11 + fs) %% 12 + 1
   m <- month(x)
   quarters <- rep(1:4, each = 3)
   s <- match(m, shifted)
   q <- quarters[s]
   if (with_year) {
      uq <- quarters[m]
      ifelse(m <= fs, ((year(x)-1) + q/10), (year(x) + q/10))
   }
   else q
}

[vspinu]

This issue slipped my radar as I forgot to re-open it back in 2018. It's fixed in the master and will be released to CRAN this week.