Reverse dependencies: any issues?

[mtennekes]

There are 52 reverse dependencies (all variants: c("Depends", "Imports", "LinkingTo", "Suggests", "Enhances")):

> x = tools::CRAN_package_db()
> y = tools::package_dependencies("tmap", which = "all", recursive = FALSE, reverse = TRUE)
> dput(y$tmap)

c("abmR", "abstr", "bangladesh", "bayesEO", "bdl", "bigDM", "blockCV", 
"CAST", "CopernicusDEM", "CvmortalityMult", "disdat", "edbuildmapr", 
"epm", "eSDM", "findSVI", "geocmeans", "glottospace", "GREENeR", 
"happign", "igr", "LabourMarketAreas", "MainExistingDatasets", 
"mapboxapi", "mapping", "MazamaSpatialPlots", "MultiscaleDTM", 
"ofpetrial", "OpenLand", "pct", "ppcSpatial", "prioriactions", 
"rGhanaCensus", "rnaturalearth", "roads", "rsat", "rstac", "sf", 
"simodels", "sits", "SpatialKDE", "SpatialRDD", "spatialreg", 
"spatialrisk", "spdep", "spnaf", "spNetwork", "stats19", "stplanr", 
"tipsae", "tmaptools", "waterquality", "zonebuilder")

I'll contact each maintainer with the request to check compatibility and to ask if there are any issues and/or questions.

Package maintainer: in case you have questions or found issues, please ask them here.

[jkennedyie]

Hi. For info I noticed::

• in tm_shape() my ext param was no longer passed to tmaptools::bb() so I now call bb() explicitly to create a bbox. Added tmaptools to package suggests just in case.
• the tm_shape() help says bbox param only used if is.main = TRUE, but bbox was used in my case when is.main was default NA.
• I replaced my tm_polygons() etc fill and alpha parameters with 4.0.0 terminology.
• typo in tm_shape() help: bbox | Bounding box of he map ... should be the map I guess.

Good luck with the final push

[mtennekes]

Thx @jkennedyie see latest commit: I've enabled the passing by of bb() arguments: e.g. tm_shape(World[1,], ext = 2) + tm_polygons(), and also updated the docs.

[mtennekes]

Imho, it would be a good idea to have both syntaxes up- and running like this:

if (packageVersion("tmap") > "3.99") {
  # tmap4 code
  tm_shape(World) + 
    tm_polygons("HPI", 
      fill.scale = tm_scale_intervals(breaks = seq(10, 45, by = 5), values = "brewer.rd_yl_bu"))

} else {
  # tmap3 code
  data(World)
  tm_shape(World) + 
    tm_polygons("HPI", breaks = seq(10, 45, by = 5), palette = "RdYlBu") +
  tm_layout(legend.outside = T)
}

This would make the submission to CRAN easier for all of us.

[jkennedyie]

Changes in that latest commit working for me - v nice.

I was wondering about best practice during the transition...yep makes sense to me until Suggests >= 4.0.0 will work.

[ailich]

I noticed some plots from my ReadME for my R package don't work on tmap4. I plan to update to the syntax but I know the goal is to have backwards compatibility so just posting the result here. Here's a simplified version of one of the plots in both tmap3 and tmap4.

V3

library(MultiscaleDTM)
#> Loading required package: terra
#> terra 1.7.78

library(tmap)
#> Breaking News: tmap 3.x is retiring. Please test v4, e.g. with
#> remotes::install_github('r-tmap/tmap')

r<- erupt()
plot(r)

eastness<- SlpAsp(r = r, metrics = "eastness")

tm_shape(eastness) +
  tm_raster(palette = c("blue", "gray", "red"), style= "cont", title = "", breaks =  c(-1,0,1), midpoint = 0, legend.reverse = TRUE)+
  tm_layout(main.title = "Eastness", 
            main.title.position = "center",
            main.title.size=0.75)

^{Created on 2024-08-07 with reprex v2.1.0}

V4

library(MultiscaleDTM)
#> Loading required package: terra
#> terra 1.7.78

library(tmap)
#> 
#> Attaching package: 'tmap'
#> The following object is masked from 'package:datasets':
#> 
#>     rivers

r<- erupt()
plot(r)

eastness<- SlpAsp(r = r, metrics = "eastness")

tm_shape(eastness) +
  tm_raster(palette = c("blue", "gray", "red"), style= "cont", title = "", breaks =  c(-1,0,1), midpoint = 0, legend.reverse = TRUE)+
  tm_layout(main.title = "Eastness", 
            main.title.position = "center",
            main.title.size=0.75)
#> -- tmap v3 code detected --
#> [v3->v4] tm_raster(): instead of 'style = "cont"', use 'col.scale = tm_scale_continuous()' and migrate the argument(s) 'breaks' (rename to 'ticks'), 'midpoint', 'palette' (rename to 'values') to 'tm_scale_continuous(<HERE>)'. For small multiples, specify a 'tm_scale_' for each multiple, and put them in a list: 'col.scale = list(<scale1>, <scale2>, ...)'
#> [v3->v4] tm_raster(): migrate the argument(s) related to the legend of the visual variable 'col', namely 'title', 'legend.reverse' (rename to 'reverse') to 'col.legend = tm_legend(<HERE>)'
#> [v3->v4] tm_layout(): use 'tm_title()' instead of the 'main.title' argument of 'tm_layout'
#> Error in if (name %in% c("cat", "seq", "div")) {: the condition has length > 1

^{Created on 2024-08-07 with reprex v2.1.0}

[mtennekes]

thx @ailich , should be fixed now

[olivroy]

I ran the revdep check Only 3 packages erroring!

https://github.com/r-tmap/tmap/actions/workflows/recheck.yaml

Common errors

# In mapping package
mappingEU(data = euNuts2, var = "total",
           subset = ~I(nuts0_id == "ES"), facets = "nuts2")
#> Error in if (any(!dtl$sel__) || !q$drop.units) { : 
#>  missing value where TRUE/FALSE needed

# in MazamaSpatialPlots
#> Error in FUN(X[[i]], ...) : object 'projection' not found
#> Calls: <Anonymous> ... lapply -> FUN -> do.call -> <Anonymous> -> tmapShape.sf

Possibly a missing option here?

# Error: processing vignette 'LabourMarketAreas.Rmd' failed with diagnostics:
# unused argument (view.legend.position = c("right", "bottom"))

In rsat, partial matching. Seems pretty harmless

genPlotGIS: warning in tm_facets(ncol = layout[2], nrow = layout[1]):
  partial argument match of 'nrow' to 'nrows'
genPlotGIS: warning in tm_facets(ncol = layout[2], nrow = layout[1]):
  partial argument match of 'ncol' to 'ncols'

Here is the result

------- Check for regressions ------
Changes:
Package: LabourMarketAreas
Check: re-building of vignette outputs
Old result: OK
New result: ERROR

Package: MazamaSpatialPlots
Check: examples
Old result: OK
New result: ERROR

Package: MazamaSpatialPlots
Check: re-building of vignette outputs
Old result: OK
New result: ERROR

Package: MazamaSpatialPlots
Check: tests
Old result: OK
New result: ERROR

Package: mapping
Check: R code for possible problems
Old result: OK
New result: NOTE

Package: mapping
Check: examples
Old result: OK
New result: ERROR

Package: rsat
Check: R code for possible problems
Old result: OK
New result: NOTE
Error: Found potential problems
Execution halted

You can download the logs at the bottom of the page (https://github.com/r-tmap/tmap/actions/runs/11488046296)

[mtennekes]

Thx @olivroy! Issues from your chunks 2 and 3 should be fixed now: the view.legend.position argument added (redirecting to legend.position (without message yet), and nrows and ncols renamed to nrow and ncol.

Only need to fix the mapping issues in your first chunk....

[mtennekes]

and nrows and ncols renamed to nrow and ncol.

FYI @Nowosad @rsbivand please check your books as this could cause minor issues. It's about tm_facets (in v3 it also was nrow and ncol, that's why I changed it back)

[Nowosad]

Thanks @mtennekes -- I just looked at each use of tm_facets in geocompr and we consistently use "nrow" and "ncol" there (nrows and ncols were not used).

Update: actually -- ncols was used once for an internal code. (https://github.com/geocompx/geocompr/pull/1143)

[mtennekes]

Cannot reproduce the mapping issue anymore. Perhaps it is fixed already? MazamaSpatialPlotsissue: see #950

[rsbivand]

@mtennekes In SDSR, for forthcoming tmap 4, tm_facets_wrap is used, for example tm_facets_wrap(columns = 2, rows = 2), and for tmap < 4 tm_facets(free.scales = FALSE, ncol = 2). I guess that should be OK? The same holds in an spdep vignette.

[mtennekes]

@rsbivand If there are 4 facets, tm_facets_wrap(nrow = 2) should do the job. The arguments columns and rows are used by tm_facets_grid to specify the grouping variables for columns and rows respectively. Similar to the by argument of tm_facets_wrap.

[rsbivand]

OK, noted. tmap HEAD for now doesn't pass CMD check, so I'll wait before trying if necessary.

[olivroy]

Looks like we were able to resolve all reverse dependencies issues. Thanks @mtennekes for sorting everything out!