rstudio / config

config package for R
https://rstudio.github.io/config/
256 stars 27 forks source link

Issue using config with R-Studio inside .Rprofile #4

Closed Prev-I closed 3 years ago

Prev-I commented 7 years ago

If you load config inside an .Rprofile file and open R-Studio this will not load... It's as easy as add inside .Rprofile: library(config)

It's probably an R-studio bug, but have you ever experience this problems?

r2evans commented 6 years ago

Is this an example of where they suggest not loading the package with library(config)? It masks two import functions, get and merge, both of which would be significant if not recognized. If you use config::get... instead, it works (at least it does now). (Confession: I didn't try to break it, I just checked that this worked.)

jjallaire commented 6 years ago

Yeah, you should never do library(config) (because of the conflicts mentioned here)

jwijffels commented 5 years ago

Just a note that I really don't like this design decision to call a function merge ... this is so silly and could be avoided giving silly name conflicts with the base package. Either way if you choose to use merge why didn't you define it as an S3 method.

daattali commented 4 years ago

I agree it's not great to mask base::get and base::merge. Even if not attaching in a .Rprofile, it's still dangerous and can lead to bugs when you override base functions.

jjallaire commented 4 years ago

The concept is/was that you never attach config, but rather use config::get, config::merge (see docs: https://github.com/rstudio/config). Obviously though this wasn't a great decision as people will still end up calling library anyway. We can't really change this now b/c there is too much code in the field that relies on it.

On Mon, Apr 27, 2020 at 7:10 PM Dean Attali notifications@github.com wrote:

I agree it's not great to mask base::get and base::merge. Even if not attaching in a .Rprofile, it's still dangerous and can lead to bugs when you override base functions.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rstudio/config/issues/4#issuecomment-620282611, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZPR6KFREWK76X2T4CTXTROYGG7ANCNFSM4DJOZL7Q .

daattali commented 4 years ago

I agree, and I don't attach it personally.

But when you work with other people and you make sure to namespace everything that's not base, and then a change by someone else that doesn't touch your code ends up breaking it just because they attached config, you get here to see if this issue has been raised :)

I don't ever namespace base functions, but this is making me rethink that. And I dont think it's right of me to need to

daattali commented 4 years ago

That said, I understand the unfortunate "too many people use it now, cant touch it" reality

andrie commented 3 years ago

Closing. The recommended usage is to explicitly call config::get()