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

update with method for Spatrasters #26

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Conversation

spono
Copy link
Contributor

@spono spono commented Oct 26, 2023

Adapted the code to work with SpatRaster from the terra package and added the possibility to output the lines as an sf object. There's a horrible workaround to solve a transposition of the data along the second axis (upper right to lower left) but couldn't find a better alternative. Overall, it works smoothly and fast even with high resolution aerial images (tested on 1km tiles with 0.15 cm pixels).
If I'll have time, I might work on some other tools

spono added 2 commits October 26, 2023 12:39
Adapted the code to work with SpatRaster from the terra package and added the possibility to output the lines as an sf object.
There's a horrible workaround to solve a transpostion of the data along the second axi but couldn't find a better alternative. Overall, it works smoothly and fast even with high resolution aerial images (tested on 1km tiles with 0.15 cm pixels)
@jwijffels
Copy link
Contributor

Thanks for the changes, I think this requires adding R package terra and R package sf in the Suggests in the DESCRIPTION file as well as some examples in the @examples section, conditional on the availability of terra and sf.

@spono
Copy link
Contributor Author

spono commented Nov 3, 2023

ok, great: if the changes I proposed are good for you, I'll push also the other updates as soon as possible. I might have some issues with the material for the example, but let's see

@spono
Copy link
Contributor Author

spono commented Jun 12, 2024

Hi, I added an example (the same one from image.contour.detector) due to some constraints with the images I was using.

I realised that all pull requests pushed to my master ended up to this commit: be patiant, I'm not that much used to github and versioning :)
BTW, you'll find updates to image.contour.detector too, mostly related to the passage to sf/terra. It happens that running the function line by line it works but, installing the package (through remotes), the SpatRaster method is not found: I guess is related to an update of the NAMESPACE from your (?) side.

@jwijffels
Copy link
Contributor

Thanks, looks already nice, if you could rewrite the code such that we don't have dplyr as Suggest dependency and rename the argument to as_sf instead of export export_sf

spono added 2 commits June 17, 2024 16:22
changed "export_sf" parameter to "as_sf"
changed "export_sf" parameter to "as_sf"
@spono
Copy link
Contributor Author

spono commented Jun 17, 2024

by now, I only renamed the parameter but I don't have time in these days to find/test an alternative in plain R to group_by & summarize which works smoothly with sf objects

@jwijffels
Copy link
Contributor

If summarize is just a union. Wouldn't this just be using sf::st_union

@spono
Copy link
Contributor Author

spono commented Jul 1, 2024

st_union didn't seem to be appropriate due to an issue: I solved using st_combine and removing the usage of dplyr.
Nevertheless, attention is needed on one point: even if the method has been added, it is not recognised when the function is called, returning the error no applicable method for 'image_contour_detector' applied to an object of class "SpatRaster".
It seems related to the fact that the NAMESPACE is not updated with the new method. I don't know how to solve it but I found this.

@jwijffels
Copy link
Contributor

jwijffels commented Jul 2, 2024

It seems related to the fact that the NAMESPACE is not updated with the new method. I don't know how to solve it but I found this.

In order to add your functions to the NAMESPACE file, you can run Document in RStudio and also don't forget to run R CMD check. As I see the examples failed.

Can you further document your output structures + have a minimally reproducible example for image_contour_detector.RasterLayer

spono added 2 commits July 3, 2024 10:49
- added curve_ID
- fixed '@return' description
- fixed minor issues
- refined sf objects creation
- removed dependency on sp and moving it to sf
- updated plot.lsd
@spono
Copy link
Contributor Author

spono commented Jul 3, 2024

few more small updates.

the example for rasterlayer is the same as terra, but with raster instead of rast.

I added small updates to the output description but, now tht the package supports spatial data, I think it may be important to documene "better" some parameters. For example, I would add something like ; expressed in pixels at the end of the descriptions for union_max_distance and union_min_length (just to avoid confusion).

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

Successfully merging this pull request may close these issues.

2 participants