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

Implement line_segment for sf objects #482

Closed
Robinlovelace opened this issue Apr 3, 2022 · 7 comments
Closed

Implement line_segment for sf objects #482

Robinlovelace opened this issue Apr 3, 2022 · 7 comments

Comments

@Robinlovelace
Copy link
Member

Implemented in sp here: https://github.com/ropensci/stplanr/blob/v0.8.5/R/linefuns.R

@agila5
Copy link
Collaborator

agila5 commented Apr 3, 2022

Hi Robin! A few years ago I answered a question on SE that asked more or less the same question: https://gis.stackexchange.com/questions/379107/how-to-split-network-in-equal-piece-line-segments-in-r/379271#379271 Check the code there if you want.

@Robinlovelace
Copy link
Member Author

Great answer. Looking at https://r-spatial.github.io/lwgeom/reference/st_linesubstring.html st_linestring() seems ideal and sad to say I didn't know the function existed! I think the reason I put it on the backburner is because the GRASS implementation, described at r-spatial/qgisprocess#26 but, also, need to prioritise #481. One question Andrea: I'm thinking of splitting out general purpose functions, e.g. into a pkg called {sfextras} or {linefunctions} but that would be extra work so default is probably to refactor {stplanr} and describe the various use cases. Will be great to reduce the number of dependencies and functions, making it an easier pkg to maintain.

@Robinlovelace
Copy link
Member Author

Heads-up @agila5 thanks to your code sharing I've implemented a solution already. Not sure how fast it is, but should work! Pls take a look, any follow on comments/suggestions/or thumbs up welcome!

@agila5
Copy link
Collaborator

agila5 commented Apr 4, 2022

LGTM. The only problem I see is that lwgeom::st_linesubstring may return a warning for lat/long data. If you want, I will propose a fix in the afternoon.

@agila5
Copy link
Collaborator

agila5 commented Apr 4, 2022

Just kidding. I think you cannot remove the warning from lwgeom::st_linesubstring if you run it with non-projected coordinates and I'm not sure how to replicate the same behaviour with S2 😅

@Robinlovelace
Copy link
Member Author

Great comments, thanks Andrea!! P.s. are you happy to review other stplanr code changes? Should be a LOT easier to maintain after mega PR #481 is reviewed/merged.

@agila5
Copy link
Collaborator

agila5 commented Apr 4, 2022

Sure but only on the weekend 😅

Robinlovelace added a commit that referenced this issue Apr 4, 2022
Robinlovelace added a commit that referenced this issue Jun 9, 2022
* Increment version number to 1.0.0

* Remove calc_catchment

* Purge rgdal for #332

* Remove sp from quadrants.R

* Update quadrant code for #332

* Refactor n_vertices

* Remove sp deps in bb functions for #332

* Try to fix bbox_scale() issue

* sf version of line_segment(), close #482

* Remove line_length() 1

* Remove line_length() 2

* More for #332

* Remove sp versions of overline

* Remove SpatialLinesNetwork funs

* Remove src directory

* Remove sln funs

* Fix typo in vignette code

* Remove more sp support from linefuns.R

* Remove more rgeos functions - toptail

* Update docs

* Complete purge of rgeos

* Purge maptools

* More removals for #332

* Fix documentation issues

* More docs

* Completely remove raster #332

* Remove geosphere, close #332

* Remove temp version of line_segment

* Turn off windows tests

* Update startup message

* Document

* Update od2odf, depend on od pkg

* Remove unnused stringr import

* Comment out failing osrm examples

* Update onload message

* Fix typo in examples causing errors

* Remove failing examples

* Remove more failing examples

* Remove unused sp-related functions

* Remove all sp calls from package, close #332

* Purge sp

* Purge unused data objects

* Purge another package, openxlsx +deprecate fun

* Update urls for cran

* Remove annoying message

* Revert "Update urls for cran"

This reverts commit f300bbc.

* Revert "Revert "Update urls for cran""

This reverts commit 8267343.

* Revert "Remove annoying message"

This reverts commit 10f9a57.
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