CSHS-CWRA / CSHShydRology

Main package
GNU Affero General Public License v3.0
35 stars 39 forks source link

Issues with Dan Moore's vignette #25

Closed KevinShook closed 3 years ago

KevinShook commented 3 years ago
  1. There is a local path in the camp_creek_test vignette:

    wd <- "C:/cshshydrology/spatial_functions/working"

    Also there is a commented out one in ShannonFalls_spatialhydrology vignette # dem <- raster::raster("c:/Research/Shannon Falls/Map/cdem_dem_151108_134502_tif/cdem_dem_151108_134502.tif")

  2. We get a lot of warnings when using the library() function in the Vignettes, because of all of the raster functions, e.g.

    There were 15 warnings (use warnings() to see them)
    > warnings()
    Warning messages:
    1: replacing previous import ‘lubridate::intersect’ by ‘raster::intersect’ when loading ‘CSHShydRology’
    2: replacing previous import ‘lubridate::origin’ by ‘raster::origin’ when loading ‘CSHShydRology’
    3: replacing previous import ‘lubridate::union’ by ‘raster::union’ when loading ‘CSHShydRology’
    4: replacing previous import ‘raster::density’ by ‘stats::density’ when loading ‘CSHShydRology’
    5: replacing previous import ‘raster::weighted.mean’ by ‘stats::weighted.mean’ when loading ‘CSHShydRology’
    6: replacing previous import ‘raster::predict’ by ‘stats::predict’ when loading ‘CSHShydRology’
    7: replacing previous import ‘raster::aggregate’ by ‘stats::aggregate’ when loading ‘CSHShydRology’
    8: replacing previous import ‘raster::quantile’ by ‘stats::quantile’ when loading ‘CSHShydRology’
    9: replacing previous import ‘raster::update’ by ‘stats::update’ when loading ‘CSHShydRology’
    10: replacing previous import ‘raster::tail’ by ‘utils::tail’ when loading ‘CSHShydRology’
    11: replacing previous import ‘raster::stack’ by ‘utils::stack’ when loading ‘CSHShydRology’
    12: replacing previous import ‘raster::unstack’ by ‘utils::unstack’ when loading ‘CSHShydRology’
    13: replacing previous import ‘raster::head’ by ‘utils::head’ when loading ‘CSHShydRology’
    14: replacing previous import ‘raster::image’ by ‘graphics::image’ when loading ‘CSHShydRology’
    15: replacing previous import ‘raster::plot’ by ‘graphics::plot’ when loading ‘CSHShydRology’

    There are a couple of ways of getting around this. The best way is to avoid the conflicts The library() function has some options, including:

    exclude,include.only    character vector of names of objects to exclude or include in the attached frame. Only one of these arguments may be used in a call to library or require

We can use this to only include the functions that we need.

Alternatively (and this is definitely poorer style), the library() function also has the option

warn.conflicts  logical. If TRUE, warnings are printed about conflicts from attaching the new package. A conflict is a function masking a function, or a non-function masking a non-function. The default is TRUE unless specified as FALSE in the conflicts.policy option.

If we have to, we could set this to FALSE, but that's not my preference

rchlumsk commented 3 years ago

The camp creek vignette will be deleted, the Shannon Falls once should be used as the only spatial vignette (for now).

Many of the raster warnings to related to use the of @import rather than @importFrom in many other functions, so updating those in all of the other package functions will be part of the task at hand to fix all the warnings in this vignette. This is in progress on my forked version but may take some time.

rchlumsk commented 3 years ago

This issue should be closed by my recent pull request on the Dan Moore branch, perhaps with some tidying at the broader package level once merged