pacificclimate / climdex.pcic

Routines to compute ETCCDI Climdex indices
GNU General Public License v3.0
23 stars 13 forks source link

Refactor climdex.r into modular R scripts #36

Closed QSparks closed 1 month ago

QSparks commented 1 month ago

Summary

This PR refactors the original climdex.R file by breaking it into multiple smaller, focused R scripts. There are no changes to the functionality; this is purely an organizational refactor. R CMD check runs without errors, warnings, or notes and all tests pass.

Changes Made

  1. Refactoring into Modular Scripts:

    • The code has been divided into several scripts based on functionality and were organized according to this comment:
      • climdexInput_class.R: Contains the definition of the climdexInput class and its associated methods, and validation routines.
      • precipitation_indices.R
      • temperature_indices.R
      • threshold_indices.R
      • date_utils.R
      • series_utils.R
      • stats_utils.R
      • threshold_utils.R
  2. Documentation Updates

    • DESCRIPTION File:
      • Added Encoding: UTF-8 to handle ✖ roxygen2 requires "Encoding: UTF-8" error
      • Addstats and utils to imports to handle errors such as: no visible global function definition for ‘read.csv’
      • Added Roxygen: list(markdown = TRUE) and updated RoxygenNote to version 7.3.2.
    • climdex.pcic-package.R:

      • Add .registration = TRUE to the @useDynLib tag so that roxygen2 would add it the namespace.
      • Updated deprecated @docType "package" to _PACKAGE.
      • Ensured that roxygen2 Generates Correct Namespace:
      • Created with roxygen2::roxygenize(clean=T)

      resolves: #35

QSparks commented 1 month ago

The GH action to test on PR is failing for macOS-latest. This isn't related to code changes in the PR, confirmed by running the action on another previously passing PR.

Reason? All the x86 architectures were intentionally skipping the two failing tests. We aren't testing any ARM architecture.

Solution: Use an Intel runner for now and open a separate issue to support Apple Silicon. Drop macOS testing and open a separate issue Related: https://github.com/actions/runner-images/issues/9741