editFeatures on user defined background map

[tim-salabim]

Consider the following scenario:

library(mapview)
library(sf)
pol = st_cast(franconia[2, ], "POLYGON")[2, ]
cntrd = st_centroid(pol)
mapview(pol) + cntrd

The centroid lies outside the polygon which is correct in mathematical terms (at least in the current implementation of sf::st_centroid but there are cases where the user would like this point to lie inside the polygon. Obviously, mapedit could provide a nice way for a quick-fix here by enabling the user to freely drag the point to a location inside the polygon. Even though the manually adjusted position may not exactly be the centroid, it may well be a better approximation of it than a point lying outside the polygon.
To achive this, of course, the user would need to be able to use editFeatures with a user defined background map including the polygon. Hence, I think we need to change the logic of editFeatures away from argument platform towards argument map = NULL so that if NULL, we use set up a map using mapview(x) and else we use the supplied map.

I think we should follow this strategy also for selectFeatures as also then there may be scenarios where the user needs some sort of background reference features other than a map or satellite view (e.g. a raster image) to aid feature selection.

@timelyportfolio thoughts?

[tim-salabim]

e6ba857 is a first trial for user defined bg map provision

[timelyportfolio]

This makes very good sense and aligns with editMap approach. Also, I much prefer this over trying to provide arguments for map customization within editFeatures/editMap.

Off topic, but it seems editMap is quickly becoming more of an internal function, which is not something I expected at the outset :)

[tim-salabim]

I think we can also think about another approach here. Using editMap to edit features on an existing map, but then we would somehow need to specify which groups of the map to activate for editing. If that is an possible that would be a similarly nice option. I guess I approach things more from a (spatial) data perspective, hence editFeatures & selectFeatures just seem more natural to me. Though I am sure there are just as many people who would approach these things from a map-based point of view where editMap would be the obvious choice.

[timelyportfolio]

We could revisit the idea of editMap.sf.

[tim-salabim]

Also, I think that the principle difference between editMap and editFeatures should be the x to be edited. A map in the former, a feature in the latter.

[tim-salabim]

In any case, we should enable user supplied maps

[timelyportfolio]

Would you like me to extend the functionality in *Features to use + or addFeatures in the case where user supplies mapview map?

[tim-salabim]

addFeatures would be better as it can also be used for leaflet map. + is only defined for mapview objects

[timelyportfolio]

For now, I will add logic to get @map if mapview is provided for map.

[timelyportfolio]

Don't do this since it crashed my session, but I just discovered issue in case of editFeatures(breweries, mapview(breweries). Should this be handled in mapview::addFeatures or mapedit?

[tim-salabim]

What's the issue? I had no problems with this example (with a fresh devtools::install_github("timelyportfolio/mapedit")

[timelyportfolio]

What happens when you mapview(breweries)@map %>% addFeatures(breweries)?

[tim-salabim]

[tim-salabim]

This looks fine to me. Identical to mapview(breweries)@map %>% leaflet::addCircleMarkers(data = breweries)

[tim-salabim]

@timelyportfolio Is this still causing problems on your side?

[timelyportfolio]

I guess my machine was just tired that day, since I no longer get problems. However, should we attempt to handle the case where x in editFeatures is already in the map provided like in editFeatures(breweries, mapview(breweries)?

Honestly, I sort of like the duplicated points from the example above that we can use as reference as we edit, so maybe we just assume user intentionally duplicated.

[tim-salabim]

Yes, I don't think we should try to be smarter that necessary. And I am sure there will be cases where scenarios like this one are completely intentional.

[timelyportfolio]

Ok, going to close. Please reopen if there are lingering unresolved issues.

[tim-salabim]

So, there are no lingering issues somewhere in mapview regarding this? I'd like to submit mapview to CRAN very shortly (possibly tonite) but want to make sure that mapedit will work as expected with that version.

[timelyportfolio]

I think it is good to go. After mapview up on cran then we can focus on mapedit cran submission. I have been running checks on eac change for the last two weeks so hopefully no surprises.

[tim-salabim]

That was my thinking. I just submitted to winbuilder, so let's see...