Recent travis failures caused by a sudden failure in checksums, which now produces different hash values:

> checksums(m, tmpdir)
Checking local files...
Finished checking local files.
# A tibble: 2 x 7
# Groups:   expectedFile [2]
  result       expectedFile         actualFile       checksum.x       checksum.y
   <chr>              <chr>              <chr>            <chr>            <chr>
1   FAIL            DEM.tif            DEM.tif 77c56d42fecac5b1 70211e468ce3d79b
2   FAIL habitatQuality.tif habitatQuality.tif f21251dcdf23dde0 70211e468ce3d79b
# ... with 2 more variables: algorithm.x <chr>, algorithm.y <chr>

from #34, assigned to @CeresBarros

fixed in 6feec34 but forgot to update NEWS

Based on a comment written on SpaDES google group

Not sure what suddenly triggered these but there appears to be an issue with caching. The same error is causing failures in vignette building:

 argument "what" is missing, with no default

checking tests ... ERROR
  Running ‘test-all.R’ [86s/90s]
Running the tests in ‘tests/test-all.R’ failed.
Last 13 lines of output:
  ══ testthat results  ═══════════════════════════════════════════════════════════
  OK: 63 SKIPPED: 33 FAILED: 36
  1. Error: test cache (@test-cache.R#14) 
  2. Error: test event-level cache (@test-cache.R#69) 
  3. Error: test module-level cache (@test-cache.R#125) 
  4. Error: test .prepareOutput (@test-cache.R#201) 
  5. Error: test .robustDigest for simLists (@test-cache.R#259) 
  6. Error: test checkpointing (@test-checkpoint.R#26) 
  7. Error: test checkpointing with disk-backed raster (@test-checkpoint.R#73) 
  8. Error: /Users/travis/build/PredictiveEcology/SpaDES.core/SpaDES.core.Rcheck/00_pkg_src/SpaDES.core/man/moduleMetadata.Rd 
  9. Error: /Users/travis/build/PredictiveEcology/SpaDES.core/SpaDES.core.Rcheck/00_pkg_src/SpaDES.core/man/moduleVersion.Rd 
  1. ...
  
  Error: testthat unit tests failed
  Execution halted

See https://cran.r-project.org/web/checks/check_results_SpaDES.core.html

* checking dependencies in R code ... NOTE
Namespaces in Imports field not imported from:
  ‘CircStats’ ‘RandomFields’ ‘grDevices’ ‘sp’
  All declared Imports should be used.

I'd like to ensure crayon colours for messages are used consistently (e.g., consistent colours for different types of messages). Should be documented in the wiki dev docs.

From @eliotmcintire on December 9, 2016 0:39

           browser()

            if (!is.null(attr(default, "unit"))) {
              browser()
              sim <- get("sim", envir = sys.frames()[grep(sys.calls(), pattern="^defineModule")][[1]])
              eval(parse(text = "convertTimeunit(deparse(substitute(default)), 'seconds', [email protected])"), envir=sim@.envir )
              eventTimeInSeconds <-
                convertTimeunit(default, "seconds", sim@.envir) %>%
                as.numeric()
            }

Copied from original issue: PredictiveEcology/SpaDES#325

Allow module developer to optianally use moduleName/moduleName_data.Rmd file to document data.

Using this feature in SpaDES.shiny (PredictiveEcology/SpaDES.shiny@7fb05ae).

From @eliotmcintire on December 16, 2016 22:24

inside downloadData, the whole structure assumes that the sourceURL can provide a filename for the local file once downloaded. If this is a dropbox file, google drive file, or short URL, or many other cases, this will not work.

Likely this requires a 4th column in the CHECKSUMS.txt file, indicating the sourceURL which goes with the local filename

The culprit line where it breaks is this:

id <- which(chksums$expectedFile == basename(x))
        

Copied from original issue: PredictiveEcology/SpaDES#327

Hi,

It seems that the code checking fails to parse the code of one of the module because it's using %>% internally.

FWI,
parse(text = "%>%") # doesn't work
parse(text = "`%>%`") # works

Edit: looks like %>% is working fine. Maybe backquotes?

See recently updated https://adv-r.hadley.nz/s4:

• use constructors instead of new():

  It’s also possible to create an instance using new() and the name of the class. This is not recommended because it introduces some ambiguity. What happens if there are two packages that both define the Person class?

• don't use { in generics:

  Note that it is bad practice to use { in the generic function. This triggers a special case that is more expensive, and generally best avoided.

• use of setOldClass():

  If you’re trying to build an S4 class on top of a S3 class provided by a package, it is better to request that the package maintainer add this call to the package, rather than running it yourself.

pkgdown::build_site() completes successfully but generates the following warnings:

Warning messages:
1: In matrix(cell_contents, ncol = length(align), byrow = TRUE) :   data length [31] is not a sub-multiple or multiple of the number of rows [16]
2: In matrix(cell_contents, ncol = length(align), byrow = TRUE) :   data length [10] is not a sub-multiple or multiple of the number of rows [4]

A warning:

> pkgdown::build_site(examples = FALSE)
Warning: Failed to parse usage:

.emptyEventListDT

.singleEventListDT

.emptyEventList(eventTime, moduleName, eventType, eventPriority)

\S4method{.emptyEventList}{numeric,character,character,numeric}(eventTime,
  moduleName, eventType, eventPriority)

\S4method{.emptyEventList}{missing,missing,missing,missing}()

.emptyEventListCols

and an error:

...
Reading 'man/robustDigest.Rd'
Reading 'man/runCodeChecks.Rd'
Error in rep(TRUE, length(x) - 1) : invalid 'times' argument

From @achubaty on November 7, 2016 23:13

Although the files exist locally, downloadModule is still checking for the existence of the module remotely for download.

This is a bigger problem with GitHub's API rate limits because only a few calls to downloadModule are needed to trigger a 403 error. The checkModule() call early in the downloadModule() function is being called without first checking to see whether files exist locally. It's worth implementing a local check/checksum for module code, in addition to the checks for downloading data.

Copied from original issue: PredictiveEcology/SpaDES#319

from https://groups.google.com/forum/#!topic/spades-users/Wk4tKdEts3o

It's worth highlighting why a Copy of the simList is being made when Caching.

From @achubaty on March 31, 2015 23:6

would be useful to maintain a list of triggers for certain events.

at minimum, a data.table with columns:

Copied from original issue: PredictiveEcology/SpaDES#143

From @achubaty on March 31, 2015 23:11

• standardized & better integration with other systems
• lower overhead

Copied from original issue: PredictiveEcology/SpaDES#144

From @achubaty on September 16, 2016 14:45

@eliotmcintire

• update checkParams() to include checks for P(sim) usage
• fix false positives when lines are commented out (include a grep for whitespace + # at the beggining of a line)
• when a parameter is defined within the parameters list (and passed to simInit) that does not exist in the module metadata, the show method for the simList object does not work. See the mapply call of simList-accessors.R (was line 56) .

Copied from original issue: PredictiveEcology/SpaDES#302

From @achubaty on October 14, 2016 16:31

@eliotmcintire

Is it worth restructuring modules to hide the doEvent.modulename and associated if/else contructs and use a defineEvent function instead?

That makes the module structure MUCH simpler, and makes adding a new event very easy: just add a new defineEvent anywhere in the file.

We can do something similar for the .inputObjects piece with a defineInit.

This would be a major structural change but would make module creation vastly easier for the user/developer.

Copied from original issue: PredictiveEcology/SpaDES#316

From @eliotmcintire on May 13, 2015 17:8

Known places:

• When an object is missing, usually not created from a different module:

  Error in (function (classes, fdef, mtable) :
  unable to find an inherited method for function ‘spTransform’ for signature ‘"NULL", "CRS"’

Copied from original issue: PredictiveEcology/SpaDES#183

After our discussion today, I realized we could add a package option spades.debug to allow a user/developer to use their preferred debug setting in when calling spades() with debug.

For developers who prefer debug = TRUE to be default, they can set the option once in their session rather than having to specify debug manually each time.

Would this be a useful addition?

From @achubaty on November 8, 2016 22:57

I ran into this issue while dealing with #319.

For non-parent modules (i.e., child modules), it is straightforward to download a particular version of a module, which is essential for reproducibility.

downloadModule("fireSpeadLcc", version = "1.1.0") ## always get v1.1.0
downloadModule("fireSpeadLcc", version = "1.1.1") ## always get v1.1.1

But the version for a parent module doesn't quite work the same way, because we are not tracking the versions of the children!

downloadModule("LCC2005", version = "1.1.1") ## always get v1.1.1 of parent
                                             ## but unknown versions of children!

While this does retrieve v1.1.1 of the parent module, depending on when it's run you will get different child module versions (because parent modules always download the most recent version of their children). This morning you would get fireSpreadLcc v1.1.0 but this afternoon that child module was bumped to v1.1.1.

This means that using parent modules is currently irreproducible out-of-the-box!

@eliotmcintire we need to record the version of the children in the parent module's metadata, which will require several tweaks:

1. using the existing version slot, allow a named list/vector of numeric_versions to be stored
2. ensure that downloadModule properly matches child version to that read from the parent's metadata

Copied from original issue: PredictiveEcology/SpaDES#321

From @achubaty on March 31, 2015 23:0

check for and message the user re: the following

• checking extents of maps
• checking the validity timeframe of modules
• deeper object name/class checking, especially global params

Most of this will need to be done on a per-module basis, with decisions being made by the module developer, but it could be useful to bake in some of this functionality

Copied from original issue: PredictiveEcology/SpaDES#142

misses local variables passed as character and evaluated later and variables inside loops

Not really a big deal, just something I noted. Whenever I am using eval(parse(text = "variable"))), simInit() doesn't recognize the variable as such and throws a warning during module checking. Not such a big deal, but you might want to fix it at some point.

The checking is also suggesting some of my created lists are not being used, but it is not recognizing I am actually using them inside a loop.

incorrectly identifies named lists as objects

Module code checking identifies named lists as objects when defaults are created in .inputObjects:

inputObjects = bind_rows(  
    expectsInput(objectName = "birdList", objectClass = "list",  
                  desc = "Example of a list of bird species containing random values")  
)  

.inputObjects <- function(sim) {
  if (!suppliedElsewhere("birdList", sim)){
  sim$birdList <- list()
  sim$birdList[["BBWA"]] <- runif(n = 10, min = 5, max = 10)
}

This also happens in the events. In the same way, it does not recognize a named list created in the module:

outputObjects = bind_rows(
    createsOutput(objectName = "predictRas", objectClass = "list", 
                  desc = "List of years, which is a list of species with density rasters")
  )
doEvent.predictBirds = function(sim, eventTime, eventType) {
  switch(
    eventType,
    init = {
      sim$predictRas[[paste0("Year", time(sim))]] <- functionX()
      # schedule future event(s)
      sim <- scheduleEvent(sim, time(sim), "predictBirds", "predictBirdsDensities")
    }, (...)

After running simInit:

###### Module Code Checking - Still experimental - please report problems ######## 
C:/Users/tmichele/GitHub/borealBirdsAndForestry/modules/predictBirds/predictBirds.R
predictBirds: inputObjects: BBWA is assigned to sim inside .inputObjects, but is not declared in inputObjects
predictBirds: module code: predictRas is declared in outputObjects, but is not assigned in the module

[ UPDATE ]

Temporary variables used inside the lapply function (i.e. variable x in lapply(X = 1:3, FUN = function(x){ })) are also pointed out as objects that are not declared in inputObjects.

###### Module Code Checking - Still experimental - please report problems ######## 
C:/Users/tmichele/GitHub/borealBirdsAndForestry/modules/birdDensityTrends/birdDensityTrends.R
birdDensityTrends: inputObjects: nSp, nRas are used from sim inside .inputObjects, but are not declared in inputObjects

Currently, depends(sim)@dependencies is an unnamed list if there is >1 module and a named list if there is 1 module, and it actually is missing a level. This should:

1. Always be a 2 level, nested list (moduleName --> dependencies) even when only 1 module
2. Add a name attribute (module name) that that list. Currently it is unnamed.

From @achubaty on April 24, 2015 21:1

indicate which modules "enhance" exisitng modules through use of modules designed to be used together (see #176).

these optional deps should be indicated by dashed line in depsGraph plot.

Copied from original issue: PredictiveEcology/SpaDES#180

SpaDES.tools is on CRAN now, but check https://cran.r-project.org/package=SpaDES.tools in a few days to ensure binaries are built for the package on Windows and macOS.

I think this might be only happing in my computer?
The download works fine, but the .zip file not actually saved inside tempdir(), which makes unzip fail.
Could someone try it on their side?

Note: I have full admin rights to my computer

## doesn't work:
tmpdir <- tempdir()
CA_admin <- prepInputs(url = "http://www12.statcan.gc.ca/census-recensement/2011/geo/bound-limit/files-fichiers/gpr_000a11a_e.zip",
                       destinationPath = tmpdir)
## works:
tmpdir <- "D:/tmpdir"
CA_admin <- prepInputs(url = "http://www12.statcan.gc.ca/census-recensement/2011/geo/bound-limit/files-fichiers/gpr_000a11a_e.zip",
                       destinationPath = tmpdir)

From @achubaty on September 16, 2016 14:23

From Glenn Sutherland:

If an event is defined, but never scheduled, ... it will never appear in the events(), completed() or current() lists. The developer therefore figures out by:

1. something isn't being done as expected
2. by inspecting the various event queues, and noticing the absence of a particular event.

This is fine, but in effect you have to infer (by subtraction) that something isn't happening that you intended to be happening, and that the problem is you failed to schedule it. ...

Suggestion: Since SpaDES know the event is there via simInit(), why not have it explicitly make a default state of "NS", and have it appear in the events() queue with that state, if in fact it has not been scheduled. This would make the problem immediately apparent, and is therefore more transparent.

Copied from original issue: PredictiveEcology/SpaDES#301

@CeresBarros suggested an overwrite argument. It would have to be passed through to GET to httr::write_disk which actually does the writing to disk.

Perhaps like this:
(https://yokekeong.com/download-files-over-https-in-r-with-httr/)

e.g.,

    library(httr)
    url <- "https://rawgit.com/yoke2/dsxref/master/iris.xlsx"
    GET(url, write_disk("iris.xlsx", overwrite=TRUE))

From @eliotmcintire on April 2, 2015 23:14

Copied from original issue: PredictiveEcology/SpaDES#149

for the argument object we read:
"Character vector or sim object in the form sim$objName"

To me, this suggests that

mySim <- simInit()
mySim$test <- 1
suppliedElsewhere("test", mySim) 

and

suppliedElsewhere(mySim$test, mySim) 
## or perhaps
suppliedElsewhere(sim$test, mySim) 

should all be TRUE. But this is not the case. This also FALSE:
suppliedElsewhere("mySim$test", sim = mySim)

Bottow line, the documentation needs to be clarified here.

From @achubaty on August 12, 2014 15:50

Copied from original issue: PredictiveEcology/SpaDES#42

currently, tests failing due to warning during checks because r-devel is stricter about R versions. Use 3.3.0 as dependency instead of 3.3.3 if possible.

* checking DESCRIPTION meta-information ... WARNING
Dependence on R version ‘3.3.3’ not with patchlevel 0

From @achubaty on March 14, 2017 20:4

Was removed in a2e46f618a5d789baa953bf836871a2b60489251 because the system call caused AWS to give an "out of memory" error and crash.

Need to identify whether this is an issue with R on systems with user limits enabled, or perhaps something else.

Copied from original issue: PredictiveEcology/SpaDES#338

While solving #2, rezipping the test module omitted the data/CHECKSUMS.txt file. I had to manually add it to the zip.

• dplyr causes problems:

  > library(dplyr)
  > rename(iris, petal_length = Petal.Length)
  Error: `petal_length` = Petal.Length must be a symbol or a string, not formula

  see tidyverse/dplyr#3232

• use of select_, mutate_, etc. is now deprecated.

From @achubaty on February 13, 2018 17:9

From my email to @eliotmcintire and @ygc2l re: PredictiveEcology/LandWeb#8

Based on discussions with Eliot, prepInputs worked with a url being passed so it doesn't need to rely on webDatabases at all. The urls for many of these files are given by sourceURL in the module metadata -- presumably those should be used by the module for downloading so that the developers don't need to change urls in multiple places.

As is, prepInputs circumvents downloadData. I understand it's doing more than simply downloading, but it's using an entirely different mechanism for the download part [downloadFromWebDB]. At the very least we can try to be consistent in where those to download mechanisms get their data.

As I see it there is a disconnect between these two download mechanisms. I feel they should both use the same mechanism for downloads to ensure that a user always gets the same result.

Copied from original issue: PredictiveEcology/SpaDES.tools#16

From @achubaty on October 11, 2016 19:42

Commits 90624d9c and 5c6f83d8 implement two functions to get and set the default path options (i.e., global defaults for simulation working directories).

1. The current implementation of setPaths won't let you change paths one at a time -- it resets the paths you're not trying to set to the defaults (fixed in PredictiveEcology/SpaDES@9bfb8d5).

   Once this is fixed, setPaths() with no arguments becomes redundant with getPaths(), so I can see it being removed and setPaths being renamed. Is defaultPaths the best name for this function??

2. Non-standard R options set by the user (e.g., those set by R packages) are not persistent across sessions. Therefore, the user needs to reset the default paths each time they load R, so our documentation needs updating to show loading the package as follows:

   library(SpaDES)
   setPaths()

Copied from original issue: PredictiveEcology/SpaDES#314

https://cran.r-project.org/web/checks/check_results_SpaDES.core.html

A few warnings and one error with latest version (0.1.1) need to be addressed ASAP.

We use the GITHUB_PAT for downloads and make note of it in the downloadModule documentation:
https://github.com/PredictiveEcology/SpaDES.core/blob/master/R/module-repository.R#L245-247

BUT the (recently added) previous paragraph says that only public repos work.
https://github.com/PredictiveEcology/SpaDES.core/blob/master/R/module-repository.R#L242-243

• improve documentation to explain how to set up the PAT and use it locally

From @achubaty on November 3, 2016 17:18

See here for a nice overview of how it could be used.

future package here: https://github.com/HenrikBengtsson/future

I can see this coming in handy for downloads, GIS operations, etc. which would normally block the main process -- these can be computed in a new process.

Copied from original issue: PredictiveEcology/SpaDES#317

Currently unable to run lint checks due to error parsing module-template.R file:

> lintr::lint_package(cache = TRUE)
........................Error in doc_parse_raw(x, encoding = encoding, base_url = base_url, as_html = as_html,  : 
  StartTag: invalid element name [68]