reproducible.destinationPath not set when supplying inputPath to paths in setupProject

[achubaty]

currently need to manually set this in order to get modules to look in 'inputs':

out <- SpaDES.project::setupProject(
  paths = list(projectPath = prjDir,
               packagePath = pkgDir,
               modulePath = "modules",
               inputPath = "inputs",
               outputPath = "outputs"),
  options = list(
    reproducible.destinationPath = "inputs", ## should be set with paths
  )
)

[eliotmcintire]

destinationPath is not a top-level path, I believe. It is commonly very idiosyncratic to modules. It seems like this is OK to leave this as is. If you or a module wants a global destinationPath, it can be set through the options. I guess it could be an optional paths ... ? i.e,. if a user wants to set it, they can... if not set, then nothing is set?

[achubaty]

We commonly use the following construct in modules to ensure inputs get to the correct location (e.g., the module's data/ folder, or a top-level inputs/ directory):

.inputObjects <- function(sim) {
  dPath <- asPath(getOption("reproducible.destinationPath", dataPath(sim)), 1)
  message(currentModule(sim), ": using dataPath '", dPath, "'.")

  ...
  
  return(invisible(sim)
}

At the top (project) level, setting inputPath = "inputs" isn't useful if modules won't use that location because it's not set with the option. (This leads to inconsistent/confusing data download and other behaviour.)

[eliotmcintire]

I don't understand the problem. destinationPath has been explicitly designed to be a module-level thing (indeed, it is likely the only one that we think of that way). So, we can do it at the top level, but it is certainly possible that one would not. In other words, with that line you showed, we should NOT set it automatically or else the dataPath will never get used. It needs to be stay unset, unless a user knows what they are doing (i.e., an advanced user). So, I think it is the right thing to leave it out of the paths argument. But I am OK if you want to put it in as a conditional ... i.e., it should stay unset if a user doesn't manually set it.

[achubaty]

Not setting reproducible.destinationPath option results in module data being put in the module's data/ directory even when the user has explicitly passed inputPath.

What is the point of using inputs/ at all if modules won't use it, unless they are all updated to use inputPath(sim) directly (but that introduces other issues). If you feel that, by default, all input data should stay in the modules' data/ subdirectory, then the default inputPath should be NULL/NA to specify this, rather than suggesting to the user that inputs/ (the current default) will be used for anything.

[eliotmcintire]

inputPath is unrelated to destinationPath, thus their names being different. If you choose to make them the same, that is a user choice and it may work fine for the user. Obviously it won't work fine if different modules save download the same file and do different things with it. This is why they are unrelated. In my opinion, it would be dangerous to have destinationPath be the same as inputPath.

destinationPath is a module-specific path (developer). inputPath is a user specified path. These should only be set the same and set at the project level if the user is VERY aware of what they are doing. I would not do this in my projects.

[eliotmcintire]

But I am OK if you want to put it in as a conditional

Please implement this within the setupPaths function, but only as a conditional, i.e., don't set anything if it isn't set explicitly by a user. It should not be related to inputPath (or inputPaths... but that is another story, of course)

[achubaty]

inputPath is unrelated to destinationPath, thus their names being different. If you choose to make them the same, that is a user choice and it may work fine for the user. Obviously it won't work fine if different modules save download the same file and do different things with it. This is why they are unrelated. In my opinion, it would be dangerous to have destinationPath be the same as inputPath.

They have used the same path in multiple projects, starting with LandWeb, and have been working great. Different modules doing different things with downloaded data is expected and appropriate. Where they could collide is if they overwrite things - but that's why we use different file names for these intermediate input/outputs (e.g. with .studyAreaName label).

destinationPath is a module-specific path (developer). inputPath is a user specified path. These should only be set the same and set at the project level if the user is VERY aware of what they are doing. I would not do this in my projects.

destinationPath is an option, set globally, so it cannot be module-specific. we have been aware of the behaviour (we designed it) and have used it across multiple projects, so it's surprising that this has changed here.

[eliotmcintire]

We have very different memories :) The reason it isn't set in setPaths is because it is not intended to be set at the simList level. The "convenience" of having an option enables you to use it in any way you want, including as you describe above. Certainly, that can be a useful strategy, and you are happily using it. However, it is not now nor has been intended to be either a globally set or simList level value. It is and has always been the only path that is different than all the others in spades that can work at the function, event, module, (simList or) or global level. It would be very cumbersome to set any of the other paths at the function, event, or module level.

I think we should keep using it in the same way we always have, i.e., flexibly and different than the others. However, I can live with it if you want to implement it in the SpaDES.project::setupPath mechanism alongside the others. It will not impact the SpaDES.core implementation.

[eliotmcintire]

@achubaty did you implement this yet? I think the options were either:

1. change the modules to use inputPaths, or,
2. add a destinationPath to the simList, and allow setupProject have a paths$destinationPath) in SpaDES.core or SpaDES.project

[achubaty]

this has not been implemented because there is disagreement over why and how to implement.

this has typically been set at the project level.

1. changing all modules and the module template to use inputPath(sim) is much more work and would likely introduce backwards compatibility issues for some workflows.
2. adding an additional path to the simList that would typically be the same as inputPath seems redundant, and would still require all modules and the template be updated, which is more work and would introduce backwards compatibility issues for some workflows.

[eliotmcintire]

To he clear: I have never set it at the project level and don't want it to be set at the project level for me and my projects.

So whatever your solution, just make it backward compatible for people who aren't using it this way.