search

r-tmap / tmap

R package for thematic maps
https://r-tmap.github.io/tmap
GNU General Public License v3.0
868 stars 121 forks source link

Don't import sf and stars fully #894

Closed olivroy closed 5 months ago

olivroy commented 5 months ago

Possibly that prior to v4 release, could remove

#' @import sf
#' @import stars

tmap would potentially load faster?

I think it is fine to leave them for dev as it may be faster to iterate?

mtennekes commented 5 months ago

Good point! For those using terra they are not strictly necessary, so we don't have to import them. What takes sf some time to load is probably the dependencies (geos/proj/gdal).

Internally, the vector-based shapes are internally stored as a list of simple features, and raster-based shapes as an empty stars object (data itself is in a data.table). The preprocessing is class-dependent: terra objects are preprocessed using native terra functions, sf using sf functions, etc.

Perhaps we could use sfheaders in the first place, and only if necessary load sf. Not sure if there is a stars 'headers' package. Alternatively, we could also switch to terra as the main package if that would be lighter weight/faster, but that would take a bit of time to change (a few days).

What are your ideas/opinions?

olivroy commented 5 months ago

Interesting! I was actually thinking of something simpler:

I think that avoiding

#' @import sf
#' @import stars

seemed to decrease load time. (not done, since it could harm development workflow) but what I propose is

Using 

#' @importFrom sf st_geometry sf_sf st_as_sf

and namespacing calls with sf:: could help. I did this in #893

mtennekes commented 5 months ago

Done importFrom. Not sure if the loading time is less...

olivroy commented 5 months ago

Oh, it seems that you are right. I must have dreamt something yesterday! Feel free to revert if you find it a burden