-
Notifications
You must be signed in to change notification settings - Fork 292
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
Add st_crop() #720
Comments
> st_bbox(c(xmin=1, ymin=1, xmax=2, ymax=2), crs = 4326)
xmin ymin xmax ymax
1 1 2 2 What is the benefit of having an |
So, just to complete the above, this also works: st_intersection(nc, st_as_sfc(st_bbox(c(xmin=-82, ymin=35, xmax=-80, ymax=36), crs=st_crs(nc)))) The proposed approach, e.g.: st_crop(nc, c(-82, -80, 35, 36)) Is more intuitive, cleaner, easier to understand and to remember. Also, it is much more easily discoverable looking through the manual and the functions provided by |
I see your point. Would you agree that requiring a named vector, as in st_crop(nc, c(xmin = -82, xmax,= -80, ymin = 35, ymax = 36)) is better, in order to avoid the mess of bbox parameters order (across packages)? |
Personally I would prefer simplicity (it's a lot of typing!) and assume the same order as > extent(c(-82, -80, 35, 36))
#or
> extent(-82, -80, 35, 36)
class : Extent
xmin : -82
xmax : -80
ymin : 35
ymax : 36 but I understand you might want to keep in line with the syntax of |
Requiring names does no longer require any order. |
I totally see why that would be more convenient given that different packages assume different orders in creating boxes and extents, I'm fine either way. |
Thanks! |
Getting: > st_crop(nc, c(xmin = -82, xmax= -80, ymin = 35, ymax = 36))
although coordinates are longitude/latitude, st_intersection assumes that they are planar
Error: nrow(x) == length(value) is not TRUE |
Hi, on this, I think it could be useful also to pass a generic "spatial" object as the second argument, from which the "cropping bbox" would be automatically computed. Automatic reprojection of the cropping bbox to the CRS of the first argument could also be useful, IMO. For example, I have a very similar functionality implemented here: https://github.com/lbusett/sprawl/blob/master/R/crop_vect.R What do you think? I could give it a go and do a PR, if you agree. |
@lbusett the current implementation tries to make a > methods(st_bbox)
[1] st_bbox.CIRCULARSTRING* st_bbox.COMPOUNDCURVE*
[3] st_bbox.CURVEPOLYGON* st_bbox.Extent*
[5] st_bbox.GEOMETRYCOLLECTION* st_bbox.LINESTRING*
[7] st_bbox.MULTICURVE* st_bbox.MULTILINESTRING*
[9] st_bbox.MULTIPOINT* st_bbox.MULTIPOLYGON*
[11] st_bbox.MULTISURFACE* st_bbox.numeric*
[13] st_bbox.POINT* st_bbox.POLYGON*
[15] st_bbox.POLYHEDRALSURFACE* st_bbox.Raster*
[17] st_bbox.sf* st_bbox.sfc*
[19] st_bbox.Spatial* st_bbox.TIN*
[21] st_bbox.TRIANGLE* I'm reluctant to adopt automatic reprojection; if here, there are many other places where we'd have to do this. |
... and yes, these warnings are annoying, but setting |
Oh, right: didn't notice the st_bbox call. If it already works with all those "spatial objects types" it's already good as is! Thanks! Concerning automatic reprojection: I understand. Maybe it could be worth Aborting in case of mismatching CRSs ? Otherwise, I think that due to:
passing a spatial object as the second argument would risk to give wrong results (though that would not work in the case of passing a "named array", since it is not needed to provide a CRS and it is assumed that the CRS is that of the object to be cropped)). Though maybe the Concerning the warnings: you are absolutely right. That lines of code should be removed. Did not even remember they were there. |
Thanks; this should only inherit it from |
Currently I crop
sf
objects this way:or, more briefly, using
spex
:or, most briefly, using
tmaptools
:I think anyone using spatial data and
sf
comes around doing this sooner or later, would this warrant anst_crop()
function, even though there are indeed ways to do this?The text was updated successfully, but these errors were encountered: