edzer / sp

Classes and methods for spatial data
http://edzer.github.io/sp/
128 stars 27 forks source link

avoid loading {sf} when not needed #131

Closed bastistician closed 1 year ago

bastistician commented 1 year ago

I have recently experimented with the impact of _SP_EVOLUTION_STATUS_=2 (ES) on {surveillance} when {sf} is not available, which I have mostly taken care of now. However, I'd also like to suggest two changes for {sp} to avoid loading {sf} when it is actually not needed.

  1. Trivial CRS(), i.e. missing projection information. In current sp 1.6-0,

    R-devel --vanilla -s -e 'library(sp); system.time(CRS())'

    gives me 0.00 seconds for ES 0, but 0.24 seconds for ES 2. I think CRS() should have a fast track to return(new("CRS", projargs = NA_character_))) early when both projargs and SRS_string are missing. For example, also coordinates(meuse) <- ~x+y unnecessarily loads {sf} in current ES 2.

  2. spplot() by default does not draw scales, but the internal function longlat.scales() queries is.projected(obj) anyway. To illustrate the impact without having 1. addressed, we need to use an existing Spatial object with a missing projection. For example,

    R-devel --vanilla -s -e 'library(sp); data(state.vbm, package="spData"); system.time(spplot(state.vbm))'

    gives me 0.02 seconds for ES 0, but 0.24 seconds for ES 2 because {sf} is unnecessarily loaded via longlat.scales.

My suggested code changes can currently be viewed in https://github.com/edzer/sp/compare/main...bastistician:sp:trivialCRS and I am happy to submit these as a PR if you agree with this issue.

These changes also reduce the time needed for R CMD checking sp-dependant packages including {sp} itself. I have traced how often {sf} is being loaded during a standard R CMD check of {sp}. With current sp 1.6-0 and ES 0, {sf} is loaded only once (when "checking dependencies in R code"). With ES 2 it is additionally loaded 15 times (once for the examples, as these all run in a single session, 11 x tests, 3 x vignettes). With my suggested changes, this number is reduced to 4 (1 x examples, 2 x tests, 1 x vignettes). On my system, the checking time was reduced by 10%, from 61 seconds to 55 seconds.

edzer commented 1 year ago

That seems a good idea. @rsbivand agreed?

rsbivand commented 1 year ago

Both make good sense.

edzer commented 1 year ago

Thanks - merged in https://github.com/edzer/sp/pull/132

bastistician commented 1 year ago

Great, thanks for merging!