All fields in classes are private

pvanlaake · pvanlaake · commit 7cabb766bdd1 · 2026-03-10T15:18:48.000+01:00
diff --git a/NEWS.md b/NEWS.md
@@ -3,6 +3,7 @@
 Maintenance release
 
 * Support for CF-1.13.
+* Fields in `CFTime` and `CFClimatology` classes have been made private, corresponding active fields added.
 * `CFClimatology` instances can now be extended with the `+` operator.
 * When extending a `CFTime` or `CFClimatology` instance with another such instance using the `+` operator, bounds are preserved if both instances have bounds set.
 * Documentation updated.
diff --git a/R/CFClimatology.R b/R/CFClimatology.R
@@ -44,7 +44,7 @@ CFClimatology <- R6::R6Class("CFClimatology",
       self$set_bounds(bounds)
 
       # Determine period and years from the bounds of the first offset
-      time <- self$cal$offsets2time(bounds[1L:2L, 1L])
+      time <- private$.cal$offsets2time(bounds[1L:2L, 1L])
       private$.years <- time$year
       if (all(time$day == 1L)) {
         if (all(time$month == 1L)) {
@@ -67,19 +67,19 @@ CFClimatology <- R6::R6Class("CFClimatology",
     #' @param ... Ignored.
     #' @return `self` invisibly.
     print = function(...) {
-      noff <- length(self$offsets)
+      noff <- length(private$.offsets)
       d <- self$range()
       el <- if (noff > 1L) {
         sprintf("  Elements: [%s .. %s] (average of %f %s between %d elements)\n",
-                d[1L], d[2L], self$resolution, CFt$units$name[self$cal$unit], noff)
+                d[1L], d[2L], private$.resolution, CFt$units$name[private$.cal$unit], noff)
       } else
         paste0("  Elements: ", d[1L], "\n")
 
       p <- paste0("  Period  : ", private$.period, "\n")
       if (private$.years[1L] == private$.years[2L]) y <- paste0("  Year    : ", private$.years[1L], "\n")
       else y <- paste0("  Years   : ", private$.years[1L], " - ", private$.years[2L], " (inclusive)\n")
 
-      cal <- capture.output(self$cal$print())
+      cal <- capture.output(private$.cal$print())
       cat(paste(cal, collapse = "\n"), "\nClimatological time series:\n",  el, p, y, sep = "")
       invisible(self)
     }
diff --git a/R/CFtime.R b/R/CFtime.R
diff --git a/R/api.R b/R/api.R
@@ -72,7 +72,7 @@ CFtime <- function(definition, calendar = "standard", offsets, from, to, by, len
       if (is.na(intv) || !length(intv_unit))
         stop("Argument `by` is an invalid time interval.", call. = FALSE)
 
-      doff <- intv * CFt$units$seconds[intv_unit] / CFt$units$seconds[t$cal$unit]
+      doff <- intv * CFt$units$seconds[intv_unit] / CFt$units$seconds[t$calendar$unit]
 
       # End point of the time series
       from_off <- t$offsets # There is only a single offset at this point
@@ -84,7 +84,7 @@ CFtime <- function(definition, calendar = "standard", offsets, from, to, by, len
         t + seq(from = from_off + doff, by = doff, length.out = length.out[1L] - 1L)
       } else {
         # Time series up to a finishing point
-        to_off <- t$cal$parse(to[1L])$offset
+        to_off <- t$calendar$parse(to[1L])$offset
         if (is.na(to_off))
           stop("Argument `to` must be a single character timestamp value.")
         t + seq(from = from_off + doff, to = to_off, by = doff)
@@ -124,23 +124,23 @@ CFtime <- function(definition, calendar = "standard", offsets, from, to, by, len
 
 #' @describeIn properties The definition string of the `CFTime` instance.
 #' @export
-definition <- function(t) t$cal$definition
+definition <- function(t) t$calendar$definition
 
 #' @describeIn properties The calendar of the `CFTime` instance.
 #' @export
-calendar <- function(t) t$cal$name
+calendar <- function(t) t$calendar$name
 
 #' @describeIn properties The unit of the `CFTime` instance.
 #' @export
 unit <- function(t) t$unit
 
 #' @describeIn properties The origin of the `CFTime` instance in timestamp elements.
 #' @export
-origin <- function(t) t$cal$origin
+origin <- function(t) t$calendar$origin
 
 #' @describeIn properties The time zone of the calendar of the `CFTime` instance as a character string.
 #' @export
-timezone <- function(t) t$cal$timezone
+timezone <- function(t) t$calendar$timezone
 
 #' @describeIn properties The offsets of the `CFTime` instance as a numeric vector.
 #' @export
@@ -472,7 +472,7 @@ slice <- function(x, extremes, rightmost.closed = FALSE) {
 #' e2 <- CFtime("days since 1850-01-01 00:00:00", "standard", 0:364)
 #' e1 == e2
 "==.CFTime" <- function(e1, e2)
-  e1$cal$is_equivalent(e2$cal) &&
+  e1$calendar$is_equivalent(e2$calendar) &&
   length(e1$offsets) == length(e2$offsets) &&
   all(e1$offsets == e2$offsets)
 
@@ -527,19 +527,19 @@ slice <- function(x, extremes, rightmost.closed = FALSE) {
 "+.CFTime" <- function(e1, e2) {
   diff <- NULL
   new_time <- if (inherits(e2, "CFTime")) {
-    if (!e1$cal$is_compatible(e2$cal)) stop("Calendars not compatible", call. = FALSE) # nocov
-    if (all(e1$cal$origin[1:6] == e2$cal$origin[1:6]))
-      CFTime$new(e1$cal$definition, e1$cal$name, c(e1$offsets, e2$offsets))
+    if (!e1$calendar$is_compatible(e2$calendar)) stop("Calendars not compatible", call. = FALSE) # nocov
+    if (all(e1$calendar$origin[1:6] == e2$calendar$origin[1:6]))
+      CFTime$new(e1$calendar$definition, e1$calendar$name, c(e1$offsets, e2$offsets))
     else {
-      diff <- e1$cal$parse(paste(e2$cal$origin_date, e2$cal$origin_time))$offset
-      CFTime$new(e1$cal$definition, e1$cal$name, c(e1$offsets, e2$offsets + diff))
+      diff <- e1$calendar$parse(paste(e2$calendar$origin_date, e2$calendar$origin_time))$offset
+      CFTime$new(e1$calendar$definition, e1$calendar$name, c(e1$offsets, e2$offsets + diff))
     }
   } else if (is.numeric(e2) && .validOffsets(e2)) {
-    CFTime$new(e1$cal$definition, e1$cal$name, c(e1$offsets, e2))
+    CFTime$new(e1$calendar$definition, e1$calendar$name, c(e1$offsets, e2))
   } else {
-    time <- e1$cal$parse(e2)
+    time <- e1$calendar$parse(e2)
     if (anyNA(time$year)) stop("Argument `e2` contains invalid timestamps", call. = FALSE) # nocov
-    CFTime$new(e1$cal$definition, e1$cal$name, c(e1$offsets, time$offset))
+    CFTime$new(e1$calendar$definition, e1$calendar$name, c(e1$offsets, time$offset))
   }
 
   if (!is.null(be1 <- e1$bounds) && !is.null(be2 <- e2$bounds)) {
@@ -561,13 +561,13 @@ slice <- function(x, extremes, rightmost.closed = FALSE) {
 "+.CFClimatology" <- function(c1, c2) {
   if (!inherits(c2, "CFClimatology"))
     stop("Can only merge a `CFClimatology` instance to another `CFClimatology`")
-  if (!c1$cal$is_compatible(c2$cal)) stop("Calendars not compatible", call. = FALSE) # nocov
+  if (!c1$calendar$is_compatible(c2$calendar)) stop("Calendars not compatible", call. = FALSE) # nocov
 
-  if (all(c1$cal$origin[1:6] == c2$cal$origin[1:6]))
-    CFClimatology$new(c1$cal$definition, c1$cal$name, c(c1$offsets, c2$offsets), cbind(c1$bounds, c2$bounds))
+  if (all(c1$calendar$origin[1:6] == c2$calendar$origin[1:6]))
+    CFClimatology$new(c1$calendar$definition, c1$calendar$name, c(c1$offsets, c2$offsets), cbind(c1$bounds, c2$bounds))
   else {
-    diff <- c1$cal$parse(paste(c2$cal$origin_date, c2$cal$origin_time))$offset
-    CFClimatology$new(c1$cal$definition, c1$cal$name, c(c1$offsets, c2$offsets + diff), cbind(c1$bounds, c2$bounds + diff))
+    diff <- c1$calendar$parse(paste(c2$calendar$origin_date, c2$calendar$origin_time))$offset
+    CFClimatology$new(c1$calendar$definition, c1$calendar$name, c(c1$offsets, c2$offsets + diff), cbind(c1$bounds, c2$bounds + diff))
   }
 }
 
@@ -840,13 +840,13 @@ month_days <- function(t, x = NULL) {
   stopifnot(inherits(t, "CFTime"))
 
   if (is.null(x))
-    return(t$cal$month_days())
+    return(t$calendar$month_days())
   else {
     if (!(is.character(x))) stop("Argument `x` must be a character vector of dates in 'YYYY-MM-DD' format")
 
-    ymd <- t$cal$parse(x)
+    ymd <- t$calendar$parse(x)
     if (anyNA(ymd$year)) warning("Some dates could not be parsed. Result contains `NA` values.", call. = FALSE)
-    return(t$cal$month_days(ymd))
+    return(t$calendar$month_days(ymd))
   }
 }
 
@@ -908,14 +908,14 @@ month_days <- function(t, x = NULL) {
 #' parse_timestamps(t, timestamps)
 parse_timestamps <- function(t, x) {
   stopifnot(is.character(x), inherits(t, "CFTime"))
-  if (t$cal$unit > 4) stop("Parsing of timestamps on a 'month' or 'year' time unit is not supported.", call. = FALSE)
+  if (t$calendar$unit > 4) stop("Parsing of timestamps on a 'month' or 'year' time unit is not supported.", call. = FALSE)
 
-  out <- t$cal$parse(x)
+  out <- t$calendar$parse(x)
   if (anyNA(out$year))
     warning("Some dates could not be parsed. Result contains `NA` values.") # nocov
   if (length(unique(na.omit(out$tz))) > 1)
     warning("Timestamps have multiple time zones. Some or all may be different from the calendar time zone.") # nocov
-  else if (out$tz[1] != t$cal$timezone)
+  else if (out$tz[1] != t$calendar$timezone)
     warning("Timestamps have time zone that is different from the calendar.") # nocov
   out
 }
diff --git a/man/CFtime.Rd b/man/CFtime.Rd
diff --git a/tests/testthat/test-CFtime.R b/tests/testthat/test-CFtime.R
@@ -134,20 +134,20 @@ test_that("test all variants of creating a CFtime object and useful functions",
 
 test_that("Leap years on some calendars", {
   t <- CFTime$new("days since 2025-01-01", "360_day")
-  expect_true(all(!t$cal$leap_year(c(2000:2025))))
+  expect_true(all(!t$calendar$leap_year(c(2000:2025))))
   t <- CFTime$new("days since 2025-01-01", "366_day")
-  expect_true(all(t$cal$leap_year(c(2000:2025))))
+  expect_true(all(t$calendar$leap_year(c(2000:2025))))
 })
 
 test_that("Calendar 'none'", {
   t <- CFTime$new("days since 2025-01-01", "none")
-  expect_true(inherits(t$cal, "CFCalendarNone"))
+  expect_true(inherits(t$calendar, "CFCalendarNone"))
   t <- t + 0:4
   ymd <- data.frame(year = c(rep(2025, 5), NA), month = c(rep(1, 5), NA), day = c(1, 1, 1, 2, 3, NA))
-  expect_equal(t$cal$valid_days(ymd), c(T, T, T, F, F, NA))
-  expect_equal(t$cal$month_days(), rep(NA, 12))
-  expect_equal(t$cal$month_days(ymd), rep(NA, 6))
-  expect_equal(t$cal$leap_year(ymd$year), rep(NA, 6))
-  expect_equal(t$cal$date2offset(ymd), c(0, 0, 0, 0, 0, NA))
-  expect_equal(t$cal$offset2date(1:5), data.frame(year = rep(2025, 5), month = rep(1, 5), day = rep(1, 5)))
+  expect_equal(t$calendar$valid_days(ymd), c(T, T, T, F, F, NA))
+  expect_equal(t$calendar$month_days(), rep(NA, 12))
+  expect_equal(t$calendar$month_days(ymd), rep(NA, 6))
+  expect_equal(t$calendar$leap_year(ymd$year), rep(NA, 6))
+  expect_equal(t$calendar$date2offset(ymd), c(0, 0, 0, 0, 0, NA))
+  expect_equal(t$calendar$offset2date(1:5), data.frame(year = rep(2025, 5), month = rep(1, 5), day = rep(1, 5)))
 })
diff --git a/tests/testthat/test-parse_deparse.R b/tests/testthat/test-parse_deparse.R
@@ -8,7 +8,7 @@ test_that("timestamp string parsing to offsets and deparsing of offsets to times
       offsets <- -1000:1000
       def <- paste(CFt$units$name[u], "since 1992-08-20")
       t <- CFtime(def, c, offsets)
-      time <- t$cal$offsets2time(t$offsets)
+      time <- t$calendar$offsets2time(t$offsets)
       ts <- as_timestamp(t, "timestamp")
       cf2 <- CFtime(def, c)
       tp <- parse_timestamps(cf2, ts)
@@ -18,10 +18,10 @@ test_that("timestamp string parsing to offsets and deparsing of offsets to times
 
   # Add no offsets, test return value
   t <- CFTime$new("days since 1992-08-20", "standard")
-  res <- t$cal$offsets2time()
+  res <- t$calendar$offsets2time()
   expect_equal(nrow(res), 0L)
   t <- CFTime$new("days since 1992-08-20", "utc")
-  res <- t$cal$offsets2time()
+  res <- t$calendar$offsets2time()
   expect_equal(nrow(res), 0L)
 })
 
@@ -33,7 +33,7 @@ test_that("testing calendars with leap years", {
     for (d in c("1996-01-15", "1996-02-29", "1996-04-01")) {
       def <- paste("days since", d)
       t <- CFtime(def, c, c(1:2500, 36501:39000))
-      time <- t$cal$offsets2time(t$offsets)
+      time <- t$calendar$offsets2time(t$offsets)
       ts <- as_timestamp(t, "timestamp")
       cf2 <- CFtime(def, c)
       tp <- parse_timestamps(cf2, ts)
@@ -52,7 +52,7 @@ test_that("testing calendars with leap years", {
 #       offsets <- runif(10000, max = 10000)
 #       def <- paste(CFt$units$name[u], "since 1978-08-20 07:34:12.2")
 #       expect_warning(t <- CFtime(def, c, offsets)) # not ordered
-#       time <- t$cal$offsets2time(t$offsets)
+#       time <- t$calendar$offsets2time(t$offsets)
 #       ts <- as_timestamp(t, "timestamp")
 #       cf2 <- CFtime(def, c)
 #       tp <- parse_timestamps(cf2, ts)
@@ -76,7 +76,7 @@ test_that("Gregorian/Julian calendar gap in the standard calendar", {
   t <- CFtime("days since 1582-10-01", "standard", 0:3)
   ts <- as_timestamp(t)
   expect_equal(ts[4], "1582-10-04")
-  expect_false(t$cal$POSIX_compatible(0:3))
+  expect_false(t$calendar$POSIX_compatible(0:3))
 
   t <- t + 4:10
   ts <- as_timestamp(t)
@@ -90,9 +90,9 @@ test_that("Gregorian/Julian calendar gap in the standard calendar", {
   expect_equal(ts[6], "1582-10-15")
 
   expect_false(t$POSIX_compatible())            # use t$offsets
-  expect_true(t$cal$POSIX_compatible(0:100))    # use supplied offsets
-  expect_true(t$cal$POSIX_compatible(-5:100))
-  expect_false(t$cal$POSIX_compatible(-6:100))
+  expect_true(t$calendar$POSIX_compatible(0:100))    # use supplied offsets
+  expect_true(t$calendar$POSIX_compatible(-5:100))
+  expect_false(t$calendar$POSIX_compatible(-6:100))
 })
 
 test_that("Leap seconds in the utc calendar work fine", {
@@ -133,7 +133,7 @@ test_that("Fractional time parts and replace Z timezone with offset", {
     for (u in 1:4) {
       def <- paste(CFt$units$name[u], "since 1978-08-20")
       t <- CFtime(def, c)
-      p <- t$cal$parse(offsets)
+      p <- t$calendar$parse(offsets)
       expect_equal(round(p$offset %% 1, 4), res[,u])
       expect_equal(p$tz, rep("+0000", 4))
     }
@@ -147,7 +147,7 @@ test_that("Fractional time parts and replace Z timezone with offset", {
   for (u in 1:4) {
     def <- paste(CFt$units$name[u], "since 1978-08-20")
     t <- CFtime(def, "utc")
-    p <- t$cal$parse(offsets)
+    p <- t$calendar$parse(offsets)
     expect_equal(round(p$offset %% 1, 4), res[,u])
   }
 
