swcarpentry / DEPRECATED-bc

DEPRECATED: This repository is now frozen - please see individual lesson repositories.
Other
299 stars 383 forks source link

Adding caveats about setwd() to R novice lesson #658

Closed karawoo closed 9 years ago

karawoo commented 9 years ago

For our final instructor training assignment, @sritchie73 and I have added some caveats about the use of setwd() in R scripts to the Best Practices section of the novice R lessons. We point out that setwd() can cause problems if others try to run your code on their machines, and we provide an example of the type of error that can result.

jdblischak commented 9 years ago

Thanks for the contribution, @karawoo and @sritchie73. Also, I'd encourage you to join the r-discuss mailing list to stay current with the SWC R community.

I am +1 to merge. Any comments from the original authors, @sarahsupp and @tracykteal?

gavinsimpson commented 9 years ago

A couple of points. The use of "crash" is inappropriate; I doubt you mean to restrict the setwd() problems to those rare instances where you cause R to segfault. Perhaps use the term "stops with an error" or `"terminates with an error" or something like that.

I've not been a Windows user for a long time, but can't you use Unix style paths within R on Windows?

I agree with the main substantive point though; be careful with setwd(). It might be worth adding that if you do need to change working directories, setting the correct working directory at the head of the script can help alleviate the pain of being dropped into the wrong directory on an error.

Far better to teach people to start R in the correct directories. Do we have notes on this for the various OSes?

I'm -1 on merging this as it stands. Needs another pass through and edit.

karawoo commented 9 years ago

Thanks for the helpful comments, @gavinsimpson and @chendaniely. I had no idea about the forward slashes in Windows. I've updated the text accordingly; let me know if you have any other suggestions.

sritchie73 commented 9 years ago

Thanks for the feedback. I didn't realise Windows could parse Unix style paths (I thought the cross-platform way to use setwd() was to use file.path()).

sritchie73 commented 9 years ago

Is it worth suggesting that if you do use setwd(), you should precede it with a call to on.exit? i.e.:

on.exit(setwd(getwd()), add=TRUE)
setwd("some/relative/file/path")
gavinsimpson commented 9 years ago

@sritchie73 using file.path() is the recommended way to build up a file path from component parts (over say paste0()-ing them, but I don't see a problem using this either.). If you want to write an actual path, ../foo.bar.R, then you can do it just like that on Windows or the correct way but then you have to escape the backslashes.

on.exit() only works within the context of a function, so it wouldn't help cases where people wanted to use this in a script.

I would also think that

pwd <- getwd()
on.exit(setwd(pwd), add=TRUE)
setwd("some/relative/file/path")

might be safer; I forget now whether expr is evaluated immediately or only when the function exit code is run, in which case getwd() might not return what you wanted it to.

sritchie73 commented 9 years ago

You're right on both points for on.exit, thanks for all the useful feedback @gavinsimpson !

jdblischak commented 9 years ago

Thanks to @gavinsimpson for the thorough review. I think his suggestions have been successfully incorporated. Further edits to this material in future PRs are welcome. Good work @karawoo and @sritchie73.