break package into smaller components

[tim-salabim]

mapview has become quite a beast to maintain... Therefore, we should break several components out of the package into smaller packages - function (package):

• [x] cubeview (cubeview)
• [x] slideview (slideview)
• [x] plainview (plainview)
• [x] sync/latticeview (leafsync)
• [x] leaflet popup functions (leafpop)
• [x] add-ons such as addMouseCoordinates (leafmv ???)

other to-dos:

• [x] set as deprecated
• [x] set as defunct
• [x] update mapview README.md
• [x] clean remaining unused functions (@fdetsch)
• [x] delete poppendorf from mapview - now in plainview
• [ ] update/move vignettes
• [ ] once defunct, update Depends, Imports & Suggests

Likely, to avoid code duplications, we also need some sort of core package that provides functions used by any 2 of the above (incl. mapview). It might work to use plainview as the core package - needs to be imported by mapview and should be ok to be imported by cubeview and slideview. I think we need to get rid of the sf dependency in plainview (use GDALutils instead?).

With plainview now on CRAN, I will eventually figure out if it could be used as the core package to avoid code duplication. Though, I wanna address at least leafpop before submitting mapview with all functions marked as deprecated. After that, I think we can focus on cleaning things further.

UPDATE: I am now more or less convinced that the way forward is to split core functions into both plainview and leafem as there are functions and dependencies that are needed in plainview but not leafem and vice versa. mapview imports both of these so from that point of view it will not matter where they reside.

[gisma]

lazy sunday... absolutely appreciated! but you are the one who is doing it and knows better than everybody what it means... However I am not aware if the R package concept allows for something like a core-package without any other functionality. Otherwise we will be hardly able to split it without massive code duplication?

[tim-salabim]

Should be ok, see tmap and tmaptools

[gisma]

hm afaik not quite the same. tmaptools is at least offering some sort of standalone functionality. Right in the moment I don't see this part in the view-core package. However usually you are right with your assessment... So let's give a try.

[tim-salabim]

cubeview successfully moved to https://github.com/r-spatial/cubeview. The only thing left to do is to transfer the kili ndvi rasterstack from mapview to cubeview data

[fdetsch]

This is not directly associated with your question, but anyway –

Since they are no longer required, I suggest we move coords2Lines() and coords2Polygons() to Orcs and flag these functions as deprecated in mapview. How about coords2JSON(), is this one still needed? If I had to guess, these utility functions might still be of value for some sp hardliners ;-)

[tim-salabim]

@fdetsch Good idea! I am unsure about coords2JSON() at this stage, it may still be used in popupTable. Can you address the other two?

[edzer]

@fdetsch a reverse dependency check would reveal whether any CRAN packages are using these functions.

[fdetsch]

@edzer if you don't mind my asking, which reverse dependency check returns both the package name and the function used therein? I am somewhat familiar with revdepcheck::revdep_check() and the like, however, I have no clue on how to get information on the mapview functions that individual packages use.

[edzer]

Oh, I do this much more primitive, e.g. for sf I use

rm -fr /tmp/rdeps
mkdir /tmp/rdeps
cp sf_*gz /tmp/rdeps
(cd /tmp/rdeps; _R_CHECK_FORCE_SUGGESTS_=FALSE R --save -e 'r <- tools::check_packages_in_dir(".", check_args = "--no-vignettes", reverse = list(which = "most", recursive = FALSE), Ncpus = 1)')

after which I examine r and the check directories created. If you build in a .Deprecated the offending packages will show up.

[tim-salabim]

slideview should also be done https://github.com/r-spatial/slideview

[tim-salabim]

Note that for now I've simply copied over all and any bits of code needed to make the packages work. This should give a good indication of what needs to go into a potential core package.

[gisma]

Actually thats seems not to much of code at all. On the other hand a fairly special compilation of functions for a standalone package...

[tim-salabim]

Lets see what's needed to make the others work...

[tim-salabim]

leafsync should also be done https://github.com/r-spatial/leafsync

@mtennekes heads-up regarding leafsync. Can you test whether this is enough to drop the mapview dependency in tmap? This is now very lightweight, no major dependencies anymore. Should significantly improve https://github.com/mtennekes/tmap/issues/240

[tim-salabim]

CRAN seems very slow at the moment, especially for new packages. I've so far submitted leafsync (about two weeks ago) and slideview (just now), cubeview to follow next. leafsync is still in incoming/newbies after one round of feedback. So this whole process may be rather time consuming.

[mtennekes]

Thanks for letting me know. Awesome work, Tim!

[tim-salabim]

I'll ping you as soon as I get CRAN confirmation.

[tim-salabim]

leafsync on CRAN ( @mtennekes ): https://cran.r-project.org/web/packages/leafsync/index.html

slideview on CRAN: https://cran.r-project.org/web/packages/slideview/index.html

[tim-salabim]

cubeview on CRAN: https://cran.r-project.org/web/packages/cubeview/index.html

[tim-salabim]

plainview on CRAN: https://cran.r-project.org/web/packages/plainview/index.html

[tim-salabim]

leafpop on cran:

https://cran.r-project.org/web/packages/leafpop/index.html

[tim-salabim]

leafem re-submitted to CRAN after minor change request.

leafem on CRAN: https://cran.r-project.org/web/packages/leafem/index.html

[tim-salabim]

Just pushed a major update of mapview reflecting all the above mentioned breakouts. Hopefully, this will take care of most things.

Updated mapview (2.7.0) with all things marked as deprecated now on CRAN

[tim-salabim]

We now have all functions living somewhere else now marled as .Defunct. One thing I'm not sure about is whether that is the end of the road or if there is another cycle where we're supposed to completely delete these functions from the package...

Currently, we still get the following:

master > library(leafem)
master > library(mapview)
GDAL version >= 3.1.0 | setting mapviewOptions(fgb = TRUE)

Attaching package: ‘mapview’

The following objects are masked from ‘package:leafem’:

    addExtent, addFeatures, addHomeButton, addImageQuery, addLogo,
    addMouseCoordinates, addStarsImage, addStaticLabels, garnishMap,
    removeHomeButton, removeMouseCoordinates

which is informative, but not really necessary.

@edzer @rsbivand do you have experience with the process of deprecating functions?

[rsbivand]

I started splitting spdep into spdep with spatial weights and weight-based autocorrelation tests and spatialreg with model fitting functions. I had planned about 4 months of deprecation before flipping to the functions moved to spatialreg defunct in spdep, but got distracted by the Proj4 string meltdown. See https://github.com/r-spatial/spdep/issues/31 and "r-lib/pkgapi" to find reverse dependency use of functions and methods to be able to check what breaks (https://stat.ethz.ch/pipermail/r-package-devel/2019q1/003590.html).

[tim-salabim]

Thanks for those pointers @rsbivand ! My hope is to be able to completely delete the functions. Especially as mapview is likely to be used in interactive mode only anyway. But we'll see what pkgapi will tell me.

[edzer]

Deprecate them, and write the deprecation date/version in a comment in the function when you do so. When you later run into it and the package has seen a few new versions, then you throw it all out. Very satisfying.