Skip to content

Move optional dependencies from Imports to Suggests in PEcAn.data.land (#3599)#3840

Merged
infotroph merged 3 commits into
PecanProject:developfrom
omkarrr2533:fix/3599-move-optional-deps-to-suggests
Mar 6, 2026
Merged

Move optional dependencies from Imports to Suggests in PEcAn.data.land (#3599)#3840
infotroph merged 3 commits into
PecanProject:developfrom
omkarrr2533:fix/3599-move-optional-deps-to-suggests

Conversation

@omkarrr2533

Copy link
Copy Markdown
Member

Summary

Closes #3599

Moves 12 packages that are only needed for specific, optional functionality
from Imports to Suggests in PEcAn.data.land.

Packages moved from Imports to Suggests

Package File(s) Reason
doSNOW soil_params_ensemble.R Only parallel backend registration
dplR matchInventoryRings.R, Read_Tuscon.R Tree ring specific
httr gSSURGO_Query.R Only gSSURGO queries
MCMCpack soil_params_ensemble.R Only Dirichlet sampling
mvtnorm plot2AGB.R Only one rmvnorm() call
neonUtilities download_NEON_soilmoisture.R NEON-specific
neonstore extract_NEON_veg.R NEON-specific
PEcAn.benchmark load_veg.R Only one load_data() call
PEcAn.visualization InventoryGrowthFusionDiagnostics.R Only ciEnvelope()
rjags InventoryGrowthFusion.R JAGS-specific analysis
sirt extract_soil_nc.R Only Dirichlet functions
sp extract_SM_CDS.R, IC_BADM_Utilities.R Legacy spatial, replaced by sf

Changes made

  • DESCRIPTION: moved all 12 packages from Imports to Suggests
  • Added requireNamespace() guards with informative logger.severe() messages in all affected R files
  • Updated NEWS.md, CHANGELOG.md, and CITATION.cff

Testing

  • Manually verified all guards present with Select-String
  • Verified no moved packages remain in Imports

mdietze
mdietze previously requested changes Mar 3, 2026

@mdietze mdietze left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rethink where these checks are performed, fix PEcAn internal installs, and determine whether we can get by with just one package for the Dirlichet distribution (if we can, MCMCpack is the more common or the two)

Comment thread modules/data.land/R/extract_SM_CDS.R Outdated
Comment thread modules/data.land/R/extract_soil_nc.R Outdated
Comment thread modules/data.land/R/InventoryGrowthFusionDiagnostics.R Outdated
@omkarrr2533 omkarrr2533 force-pushed the fix/3599-move-optional-deps-to-suggests branch from 68cf89f to 5c194e0 Compare March 4, 2026 04:40
@omkarrr2533

Copy link
Copy Markdown
Member Author

Sir, I have also send you my GSOC proposal for the 3rd project "Increasing PEcAn Modularity" project . when you get a chance, could you take a look and share any feedback..? i know you're busy. Thanks!

@infotroph infotroph left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm excited to move all of these to Suggests! Thanks for identifying the packages to reclassify.

Before merging:

  • I see a few more places to adjust placement within the function
  • the proposed change in extract_soil_gssurgo conflicts with another open PR and should likely be deferred
  • Please update the dependency list to reflect these package changes by running ./scripts/generate_dependencies.R and committing the changes it creates (which should all be in docker/depends/pecan_package_dependencies.csv)

Comment thread CHANGELOG.md Outdated
Comment thread modules/data.land/NEWS.md
Comment thread modules/data.land/R/InventoryGrowthFusion.R
Comment thread modules/data.land/R/InventoryGrowthFusionDiagnostics.R Outdated
Comment thread modules/data.land/R/Read_Tuscon.R Outdated
Comment thread modules/data.land/R/matchInventoryRings.R Outdated
Comment thread modules/data.land/R/plot2AGB.R Outdated
Comment thread modules/data.land/R/soil_params_ensemble.R Outdated
Comment thread modules/data.land/R/soil_params_ensemble.R Outdated
Comment thread modules/data.land/R/soil_params_ensemble.R Outdated
@omkarrr2533 omkarrr2533 force-pushed the fix/3599-move-optional-deps-to-suggests branch from 5c194e0 to 3fe8164 Compare March 4, 2026 13:19
@infotroph

Copy link
Copy Markdown
Member

You now have a merge conflict in pecan_package_dependencies.csv, and from the (incorrect) structure of the changes in that file it looks like you did not use generate_dependencies.R as requested. Please fix.

…anProject#3599)

Moved doSNOW, dplR, httr, MCMCpack, mvtnorm, neonUtilities, neonstore,
PEcAn.benchmark, PEcAn.visualization, rjags, sirt, and sp from Imports
to Suggests. Added requireNamespace() guards in all affected R files.
Updated NEWS.md, CHANGELOG.md, and CITATION.cff.
@omkarrr2533 omkarrr2533 force-pushed the fix/3599-move-optional-deps-to-suggests branch from 65e70a4 to b2e6732 Compare March 5, 2026 02:43
@omkarrr2533

omkarrr2533 commented Mar 5, 2026

Copy link
Copy Markdown
Member Author

Done

@infotroph infotroph dismissed mdietze’s stale review March 5, 2026 18:11

Changes addressed

@infotroph infotroph enabled auto-merge March 5, 2026 18:12
@github-actions github-actions Bot added the tests label Mar 6, 2026
@infotroph infotroph added this pull request to the merge queue Mar 6, 2026
Merged via the queue into PecanProject:develop with commit 550a17b Mar 6, 2026
19 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move optional dependencies from Imports to Suggests

3 participants