search

ropensci / weathercan

R package for downloading weather data from Environment and Climate Change Canada
https://docs.ropensci.org/weathercan
GNU General Public License v3.0
102 stars 29 forks source link

Only import functions we need #26

Closed boshek closed 6 years ago

boshek commented 7 years ago

From goodpractices::gp(): Do not import packages as a whole, as this can cause name clashes between the imported packages. Instead, import only the specific functions you need.

boshek commented 7 years ago

I see that here you've inserted@import rather than here. If there any reason for that? I feel like having all the imports and importfroms might be better in one location? I'm not totally sure what the convention is. Any strong feelings either way?

steffilazerte commented 7 years ago

I think my rational was that I import only when really really necessary (and I only used @import lubridate to get at %within%, but if you know how to reference a special function directly, lubridate::%within%?? I'd love to do that instead). So I didn't want to use import lubridate everywhere, only when the function was referenced. No strong feelings either way, except I'd rather not use @import at all.

steffilazerte commented 7 years ago

Turns out I didn't truly understand that the @import associated with a particular function applied to the whole package (I thought it applied only to the function in question). I've fixed all imports to either be explicit (i.e. lubridate::`%within%`(x, y)) or central (i.e. put in the help.R file, as you suggested).

Fixed with e418cdd7ffac95c08f16552b30c289b3b09ccfaf