Skip to content

Refactor R package structure into domain-based modules#388

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/redo-refactor-domain-modules
Draft

Refactor R package structure into domain-based modules#388
Copilot wants to merge 2 commits intomainfrom
copilot/redo-refactor-domain-modules

Conversation

Copy link

Copilot AI commented Mar 9, 2026

Rebases the refactor from (closed) PR #384 onto current main. Pure structural reorganization — no logic changes, fully backward compatible.

Description

Reorganizes the flat 28-file R/ directory into a domain-based module hierarchy. Splits monolithic files (utilities.R at 7 fns, figures.R at 940 lines) into focused modules. Extracts magic strings into constants. Adds package-level overview docs.

New structure

R/
├── core/constants.R                  # FORECAST_FILE_*, DEFAULT_FORECAST_PATH, ZIP_OPERATION_*
├── data/data_download.R              # ← R/download.R
├── forecasting/forecast_storage.R   # ← R/zip_unzip_forecasts.R (now uses constants)
├── models/
│   ├── models_jags.R                 # ← R/models_jags.R
│   └── models_metadata.R            # ← R/prepare_metadata.R
├── utils/
│   ├── data_utils.R                  # round_na.interp, foy, named_null_list, update_list, ifnull
│   ├── file_utils.R                  # file_ext
│   ├── messaging.R                  # ← R/messages.R
│   └── package_utils.R              # package_version_finder
├── visualization/
│   ├── plots_covariates.R            # plot_covariates
│   ├── plots_diagnostics.R          # plot_forecasts_error_lead, plot_forecasts_cov_RMSE
│   └── plots_forecasts.R            # plot_forecast_ts, plot_forecast_point
└── webapp/
    ├── app_launch.R                  # ← R/webapp.R
    ├── webapp_server.R               # ← R/webapp_server.R
    └── webapp_ui.R                   # ← R/webapp_ui.R

Other changes

  • R/portalcasting.R merged into R/portalcasting-package.R (single _PACKAGE sentinel + imports + overview docs)
  • REFACTORING_SUMMARY.md added documenting what moved where
  • Bug fix included: plot_forecasts_error_lead multi-panel path referenced non-existent pevals_in$lead; corrected to pevals_in$lead_time_newmoons (the column computed earlier in the same function — required to keep that code path functional)

All 188 exports unchanged.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Not backwards compatible new feature (breaking change which adds functionality)
  • New model
  • New dataset
  • Documentation edit

How Has This Been Tested?

  • All 34 R files parse without errors (static check)
  • 188 @export directives confirmed present across new file locations
  • No duplicate function definitions across modules
  • Tests are function-based, not file-path-based — no test changes required
  • GitHub Actions CI runs on push

Checklist:

  • My code follows the style guidelines of this project
  • I follow the code of conduct for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

Review

@juniperlsimonis

Original prompt

Create a new draft pull request in weecology/portalcasting to redo the earlier refactor from PR #384 (“Refactor R package structure into domain-based modules”), but rebased onto the latest main (current main head commit bee3e6ff3c3964b0b125431cd29c93dc83fe0ac1 as of 2026-03-03).

Context:

Requirements:

  1. Open a NEW draft PR (not re-opening Refactor R package structure into domain-based modules #384).
  2. Keep it as a pure refactor wherever possible (no business logic changes).
  3. Ensure all package functions/exports remain available (exports maintained) and R CMD check/tests should still pass.
  4. Preserve / recreate the new directory structure under R/ similar to PR Refactor R package structure into domain-based modules #384:
R/
├── core/constants.R
├── data/data_download.R
├── forecasting/forecast_storage.R
├── models/{models_jags.R, models_metadata.R}
├── utils/{data_utils.R, file_utils.R, messaging.R, package_utils.R}
├── visualization/{plots_forecasts.R, plots_diagnostics.R, plots_covariates.R}
└── webapp/{app_launch.R, webapp_server.R, webapp_ui.R}
  1. Re-apply the file moves/splits that were done in PR Refactor R package structure into domain-based modules #384 (adapt as needed if upstream files changed):
  • R/utilities.R split into:
    • R/utils/package_utils.R (contains package_version_finder() docs + export)
    • R/utils/data_utils.R (contains round_na.interp(), foy(), named_null_list(), update_list(), ifnull() plus their roxygen)
    • R/utils/file_utils.R (contains file_ext() docs + export)
  • R/figures.R split into:
    • R/visualization/plots_forecasts.R (contains plot_forecast_ts(), plot_forecast_point())
    • R/visualization/plots_diagnostics.R (contains plot_forecasts_error_lead(), plot_forecasts_cov_RMSE())
    • R/visualization/plots_covariates.R (contains plot_covariates())
  • R/messages.R -> R/utils/messaging.R
  • R/download.R -> R/data/data_download.R
  • R/prepare_metadata.R -> R/models/models_metadata.R
  • R/models_jags.R -> R/models/models_jags.R
  • R/zip_unzip_forecasts.R -> R/forecasting/forecast_storage.R
  • R/webapp.R -> R/webapp/app_launch.R
  • R/webapp_ui.R -> R/webapp/webapp_ui.R
  • R/webapp_server.R -> R/webapp/webapp_server.R
  1. Keep/restore the core/constants.R and update forecast_storage.R to use constants (as in PR Refactor R package structure into domain-based modules #384):
  • DEFAULT_FORECAST_PATH
  • FORECAST_FILE_CSV, FORECAST_FILE_YAML, FORECAST_FILE_JSON
  • ZIP_OPERATION_ZIP, ZIP_OPERATION_UNZIP
  1. Add/update package docs file R/portalcasting-package.R (as in PR Refactor R package structure into domain-based modules #384) if it still makes sense with current code; ensure it doesn’t break roxygen.

  2. Keep the REFACTORING_SUMMARY.md document (or update it) describing what moved where.

  3. If any of the refactor changes in Refactor R package structure into domain-based modules #384 had incidental tiny fixes (e.g., plot_forecasts_error_lead referencing lead_time_newmoons instead of a non-existent lead column), only include them if they are necessary to keep code working on current main or if they already exist on main. Otherwise, keep refactor-only.

Deliverables:

  • A new draft PR targeting main, with a clear description summarizing the refactor and stating it is intended as refactor-only (no functional changes).
  • All moved files exist under the new module directories and old locations removed/updated accordingly.
  • CI should be green (or at least no new failures attributable to the refactor).

No images provided.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: We tried to refactor the package however the main branch has moved and has more logical changes, can you close this PR and then create a new PR with the same aim of refactoring

Assistant: I can help with the rebase/refactor and open a new PR, but I can’t close PR #384 from here with the tools I have (the available GitHub tools in this chat don’t support closing pull requests).

What I can do next

  1. Sync the refactor onto the current main (incorporating the logical changes that landed after this PR was opened).
  2. Push the updated refactor to a new branch.
  3. You (or a maintainer) closes PR Refactor R package structure into domain-based modules #384 in GitHub UI, then open a new PR from the new branch.

Before I proceed, I need 2 quick details

  1. Confirm the repo is weecology/portalcasting and the target base branch is main.
  2. What should the new PR be called / how should it relate to the old one?
    • Option A: “Refactor R package structure into domain-based modules (v2)”
    • Option B: keep the same title

If you confirm those, I’ll:

This pull request was created from Copilot chat.


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Co-authored-by: henrykironde <5192965+henrykironde@users.noreply.github.com>
Copilot AI changed the title [WIP] Redo earlier refactor for domain-based modules Refactor R package structure into domain-based modules Mar 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants