Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't import sf and stars fully #894

Closed
olivroy opened this issue Jun 25, 2024 · 4 comments
Closed

Don't import sf and stars fully #894

olivroy opened this issue Jun 25, 2024 · 4 comments

Comments

@olivroy
Copy link
Contributor

olivroy commented Jun 25, 2024

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
Copy link
Member

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
Copy link
Contributor Author

olivroy commented Jun 26, 2024

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 added a commit that referenced this issue Jun 26, 2024
@mtennekes
Copy link
Member

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

@olivroy
Copy link
Contributor Author

olivroy commented Jun 26, 2024

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

@olivroy olivroy closed this as completed Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants