r-spatial / mapedit

Interactive editing of spatial data in R
https://www.r-spatial.org/r/2019/03/31/mapedit_leafpm.html
Other
218 stars 33 forks source link

circles created with leafpm returned as points #91

Closed lbusett closed 5 years ago

lbusett commented 5 years ago

Adding a circle with the new editor actually return a point with a "radius" attribute:

polys <- st_as_sfc(c("POLYGON((0 0 , 0 1 , 1 1 , 1 0, 0 0))")) %>% 
  st_sf(ID = paste0("poly", 1))
tst = editFeatures(polys, editor = "leafpm")
#draw a circle
tst
Simple feature collection with 2 features and 3 fields
geometry type:  GEOMETRY
dimension:      XY
bbox:           xmin: 0 ymin: 0 xmax: 1 ymax: 1
epsg (SRID):    NA
proj4string:    NA
     ID layerId   radius                       geometry
1 poly1    <NA>       NA POLYGON ((0 0, 0 1, 1 1, 1 ...
2  <NA>     316 17414.39      POINT (0.340576 0.554654)

This could be maybe avoided by "Intercepting" features with "radius != NA" in the returned object and transforming them to polygons with a st_buffer of the given radius (though maybe this would require dealing with map distorsions?) . What do you think?

timelyportfolio commented 5 years ago

@lbusett we actually add the radius as a property in our htmlwidget bindings in an attempt to preserve this otherwise lost information.

I'm not sure what mapedit should do to improve this. Happy to hear thoughts.

lbusett commented 5 years ago

@timelyportfolio from a general point of view, as a "user" my expectation would be to get in the output of editFeatures an sf object that "represents" more or less what was visualized on the screen when I pressed "Done". So, I think that the circular feature should be if possible converted to a POLYGON, or a CIRCULARSTRING (do not think that CIRCULARSTRING is implemented in sf yet, though).

However, I do not think this is a big problem (also because I do not think that the ability to draw "circles" is very useful per se ;-)). Nonetheless, maybe the behaviour should be clearly documented?

tim-salabim commented 5 years ago

I think a note in the documentation should suffice unless you have a clear use-case and consider this a problem. I've tried to stay away from markers especially in mapview as much as possible as I don't like them personally. If there arises a need in the future to implement markers instead of circlemarkers we can still change it. Or when sf implements CIRCULARSTRING.

lbusett commented 5 years ago

I agree. I do not see "problems" introduced by this behaviour: it was just surprising the first time I noticed it. Documenting it should suffice IMHO. If it was for me, I would even consider removing the "create circle" feature: I can not really see a clear use case for it.

timelyportfolio commented 5 years ago

@lbusett we actually disable circle by default in the edit module with Leaflet.draw for these reasons, and I forgot. I agree we should disable by default for Leaflet.pm as well. I will make the change unless @tim-salabim says no.

timelyportfolio commented 5 years ago

going to close. Please feel free to reopen.