Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Travis fixes for R 4.0 #2592

Merged
merged 18 commits into from May 4, 2020
Merged

Travis fixes for R 4.0 #2592

merged 18 commits into from May 4, 2020

Conversation

infotroph
Copy link
Member

@infotroph infotroph commented May 1, 2020

Description

  • Completely stop using precompiled packages from C2D4U -- making them work across R versions was more headache than it was worth
  • Remove scripts/travis/rebuild_pkg_binaries.R, which was an inefficient workaround for C2D4U mixing its compilation versions
  • Use existing Docker dependencies file to preinstall needed R packages before starting PEcAn build. Probably doesn't improve runtime much, but should make cache cleanup easier and make log files easier to read and debug.
  • Add testing on R 3.5 (the release that was oldrel before), so we now test on the current release, two old versions, and the unreleased development version of R. Only failures in the released version are counted as breaking the build.
  • Typo fixes in cache buildup script: hopefully it will now run all of make install when asked.
  • Starting in R 4.0, R CMD check now complains when a package imports more than 20 dependencies. We have two such: data.atmosphere and assim.batch. We should probably listen to these complaints eventually, but for now I've ignored them by adding them to the cached check outputs for these packages.
  • Was getting a new instance of previously-ignored check message about undocumented datasets in data.atmosphere. Drafted up documentation for all the datasets, then realized we'll need to switch to lazy-loading, with possible unintended namespace changes, to use these. Left documentation commented out in data.R for future use, fixed check script to keep ignoring for now.
  • Fix long-standing code-documentation mismatch in uncertainty::plot_variance_decomposition. Checks complained now because R >= 3.6.3 notices when ... appears in documentation but not code (I assume ignoring it before was a bug).
  • Avoid leaving files behind in examples for RTM::fortran_data_module (The check for this is new in 4.0)

A point of concern: Many of the issues that break the build in 4.0 never showed up in previous R-devel releases (I went back and checked; devel builds were passing consistently before release), and while testing this most of them still don't break the R-devel build (what will become R 4.1). If these are changes implemented in 4.0 that will be sticking around, I don't understand why they aren't also showing up in devel.

Motivation and Context

Review Time Estimate

  • Immediately
  • Within one week
  • When possible

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@infotroph
Copy link
Member Author

My understanding is because of the way Travis handles PR builds[1], it will not be possible to get a passing build on this before merging. You can see the checks passing, after some manual cache-massaging that will hopefully be a one-time thing, by looking at builds for the travis_r_4 branch of either my fork or PecanProject.

[1]: Specifically: PR builds take their cache from the target branch, where none of these changes will be until it's merged.

##' Defaults to the first sensitivity analysis output given
##' @param col Color of each sensitivity analysis. Equivalent to col parameter of the plot function.
##' @param pch Shape of each sensitivity analysis. Equivalent to pch parameter of the plot function.
##' @param main Plot title. Useful for multi-pft variance decompositions.
Copy link
Member Author

Choose a reason for hiding this comment

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

@dlebauer this seems like ancient history (the function signature seems to have changed to remove these params in 2012) -- are we losing anything of value deleting these?

@mdietze mdietze merged commit 4da2f39 into PecanProject:develop May 4, 2020
@infotroph infotroph deleted the travis_r_4 branch May 4, 2020 21:26
infotroph added a commit to infotroph/pecan that referenced this pull request May 6, 2020
@infotroph infotroph mentioned this pull request May 6, 2020
12 tasks
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.

None yet

4 participants