Skip to content

Conversation

mshvartsman
Copy link

When using BatchJobs in moderately sized codebases, it can be helpful to use relative paths for both .tmpl and config files. loadConfig() uses sys.source() (via sourceConfFile()), which by default sources the config file relative to the current path, not the path of the config file itself. There are a number of ways to address this -- I think I picked the least disruptive (but maybe most awkward), which is passing arguments from loadConfig() down to sys.source(), and passing chdir=T to evaluate paths in the config file relative to its own directory.

…onfig to use relative paths for its .tmpl files, which is useful in being able to organize (bigger) codebases that rely on BatchJobs.
@mshvartsman
Copy link
Author

I should add that personally, think a better solution is to default to chdir=T on line 18 of conf.R. I see three use cases:

  1. User evaluates loadConfig() with the working directory set to the config file. Then relative template paths within the config work fine, but they would also work fine with chdir=T.
  2. User evaluates loadConfig() from another working directory, and the template path is relative to the config directory. Then loadConfig() fails to find the template without chdir=T.
  3. User evaluates loadConfig() from another working directory, but the template path is relative to that other working directory. Then loadConfig() fails to find the template with chdir=T.

The pull request as it is now doesn't change the status quo: 1 and 3 still work, 2 still breaks (but can now be fixed by the user passing chdir). My sense is that 1 and 2 are the more reasonable ones to support, but you know the BatchJobs userbase and use cases better. I am happy to revise this pull request to default to chdir=T if you prefer, with or without the ellipsis. Can also pass chdir explicitly (with default T or F) instead of ellipsis.

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.

1 participant