{"action":"reopened","issue":{"url":"https://api.github.com/repos/Robinlovelace/geocompr/issues/244","repository_url":"https://api.github.com/repos/Robinlovelace/geocompr","labels_url":"https://api.github.com/repos/Robinlovelace/geocompr/issues/244/labels{/name}","comments_url":"https://api.github.com/repos/Robinlovelace/geocompr/issues/244/comments","events_url":"https://api.github.com/repos/Robinlovelace/geocompr/issues/244/events","html_url":"https://github.com/Robinlovelace/geocompr/issues/244","id":324746914,"number":244,"title":"Review 3: responses","user":{"login":"Nowosad","id":3457131,"avatar_url":"https://avatars0.githubusercontent.com/u/3457131?v=4","gravatar_id":"","url":"https://api.github.com/users/Nowosad","html_url":"https://github.com/Nowosad","followers_url":"https://api.github.com/users/Nowosad/followers","following_url":"https://api.github.com/users/Nowosad/following{/other_user}","gists_url":"https://api.github.com/users/Nowosad/gists{/gist_id}","starred_url":"https://api.github.com/users/Nowosad/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Nowosad/subscriptions","organizations_url":"https://api.github.com/users/Nowosad/orgs","repos_url":"https://api.github.com/users/Nowosad/repos","events_url":"https://api.github.com/users/Nowosad/events{/privacy}","received_events_url":"https://api.github.com/users/Nowosad/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"Robinlovelace","id":1825120,"avatar_url":"https://avatars2.githubusercontent.com/u/1825120?v=4","gravatar_id":"","url":"https://api.github.com/users/Robinlovelace","html_url":"https://github.com/Robinlovelace","followers_url":"https://api.github.com/users/Robinlovelace/followers","following_url":"https://api.github.com/users/Robinlovelace/following{/other_user}","gists_url":"https://api.github.com/users/Robinlovelace/gists{/gist_id}","starred_url":"https://api.github.com/users/Robinlovelace/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Robinlovelace/subscriptions","organizations_url":"https://api.github.com/users/Robinlovelace/orgs","repos_url":"https://api.github.com/users/Robinlovelace/repos","events_url":"https://api.github.com/users/Robinlovelace/events{/privacy}","received_events_url":"https://api.github.com/users/Robinlovelace/received_events","type":"User","site_admin":false},"assignees":[{"login":"Robinlovelace","id":1825120,"avatar_url":"https://avatars2.githubusercontent.com/u/1825120?v=4","gravatar_id":"","url":"https://api.github.com/users/Robinlovelace","html_url":"https://github.com/Robinlovelace","followers_url":"https://api.github.com/users/Robinlovelace/followers","following_url":"https://api.github.com/users/Robinlovelace/following{/other_user}","gists_url":"https://api.github.com/users/Robinlovelace/gists{/gist_id}","starred_url":"https://api.github.com/users/Robinlovelace/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Robinlovelace/subscriptions","organizations_url":"https://api.github.com/users/Robinlovelace/orgs","repos_url":"https://api.github.com/users/Robinlovelace/repos","events_url":"https://api.github.com/users/Robinlovelace/events{/privacy}","received_events_url":"https://api.github.com/users/Robinlovelace/received_events","type":"User","site_admin":false},{"login":"Nowosad","id":3457131,"avatar_url":"https://avatars0.githubusercontent.com/u/3457131?v=4","gravatar_id":"","url":"https://api.github.com/users/Nowosad","html_url":"https://github.com/Nowosad","followers_url":"https://api.github.com/users/Nowosad/followers","following_url":"https://api.github.com/users/Nowosad/following{/other_user}","gists_url":"https://api.github.com/users/Nowosad/gists{/gist_id}","starred_url":"https://api.github.com/users/Nowosad/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Nowosad/subscriptions","organizations_url":"https://api.github.com/users/Nowosad/orgs","repos_url":"https://api.github.com/users/Nowosad/repos","events_url":"https://api.github.com/users/Nowosad/events{/privacy}","received_events_url":"https://api.github.com/users/Nowosad/received_events","type":"User","site_admin":false},{"login":"jannes-m","id":9986952,"avatar_url":"https://avatars3.githubusercontent.com/u/9986952?v=4","gravatar_id":"","url":"https://api.github.com/users/jannes-m","html_url":"https://github.com/jannes-m","followers_url":"https://api.github.com/users/jannes-m/followers","following_url":"https://api.github.com/users/jannes-m/following{/other_user}","gists_url":"https://api.github.com/users/jannes-m/gists{/gist_id}","starred_url":"https://api.github.com/users/jannes-m/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jannes-m/subscriptions","organizations_url":"https://api.github.com/users/jannes-m/orgs","repos_url":"https://api.github.com/users/jannes-m/repos","events_url":"https://api.github.com/users/jannes-m/events{/privacy}","received_events_url":"https://api.github.com/users/jannes-m/received_events","type":"User","site_admin":false}],"milestone":null,"comments":1,"created_at":"2018-05-20T22:10:04Z","updated_at":"2018-05-26T08:19:45Z","closed_at":null,"author_association":"COLLABORATOR","body":"## reviewer one\r\n\r\nChapter 9:\r\n\r\n- [ ] 1.1 [JN, RL] Section 9.2 – I agree with the authors that 'tmap' is appropriate as an introduction to cartography in R, having the least steep learning curve for production-quality standard maps. However, it is also relatively narrow in scope and specialized. When something non- standard is necessary the user will have to fall back on more general visualization approaches. It can be mentioned that 'ggplot2' and 'base'/'grid' provide a lot of flexibility which is sometimes necessary to produce advanced or novel map designs, 'ggplot2' through its powerful syntax and huge ecosystem and user community; 'base'/'grid' through their \"canvas drawing\" style of flexibility. Someone who wants to produce complex and novel types of maps will sooner or later need to go deeper into one of the latter (or both). To demonstrate this point, the authors may also consider including some inspirational examples of advanced maps produced in R, such as - ◦ http://blog.revolutionanalytics.com/2012/02/what-are-the-most-popular-bike-routes-in- london.html ◦ http://spatial.ly/2017/04/population-lines-how-and-why-i-created-it/ On the other hand, 'tmap' makes it easy to add essential elements such as a north arrow and map scale, which is less straightforward in general-purpose visualization packages.\r\n- [ ] 1.2 [RL] Section 9.2.1, 1st code section – What type of object is 'nz'? Is it an 'sf' layer? Maybe clarify right from the start what type of raster and vector classes the 'tmap' package supports.\r\n- [ ] 1.3 [RL] Section 9.2.3 – What happens when there is a column named the same way as a fixed setting? For example, suppose 'dat' has a column named \"red\", what does the expression 'tm_shape(dat) + tm_fill(col = \"red\")' do?\r\n- [ ] 1.4 [RL] Section 9.2.3 – I'm not always getting the behavior shown in the left panel of Figure 9.4. For example, running the following code - library(sf) nc = st_read(system.file(\"shape/nc.shp\", package=\"sf\")) plot(st_geometry(nc), col = nc$AREA) Produces a plot with no fill colors - [EXAMPLE IN THE PDF] Maybe you mentioned it elsewhere, if not then perhaps it will be helpful to say what kind of symbology does 'plot' produce when given a geometry and aesthetic mapping to numeric/character/units etc. data types.\r\n- [ ] 1.5 [RL] Section 9.5 – To make it clear what exactly constitutes the 'app.R' file for the 'lifeApp' example, I would include into the code section the 'library' expressions for loading the 'shiny' and 'leaflet' packages, and the expression to load the 'world' layer.\r\n- [ ] 1.6 [RL] Section 9.5 – The authors can add that, aside from dynamic queries of data to put on the map, one of the most powerful features of the shiny/leaflet combination is exploiting map events. For example, the map can be programmed to respond to mouse clicks by showing distance to some features, drawing new layers, querying information in ways other than a popup, and so on.\r\n- [ ] 1.7 [RL] Section 9.6 – Regarding 'plot' with 'sf' and 'raster' layers, perhaps mention that combining them works when both packages trigger drawing into the same plot area and do not include interfering elements. For example, plotting an 'sf' geometry and a single-band raster (as shown in the example) works well, but other combinations (e.g. multi-band raster) will not work.\r\n\r\nChapter 10:\r\n\r\n- [ ] 1.8 [JM] Section 10.1 – Consider including a brief summary on the types of tasks that each program brings, which are not currently available in R. This is given in general terms in section 10.4 but could be expanded. For example, terrain analysis will probably be done in SAGA, network analysis in GRASS, etc. Perhaps also replace the QGIS example (counting points in polygons) with a QGIS algorithm that is not available in R, or an algorithm that is much more complicated to apply in R. Otherwise unaware users may wonder why go through all the setup trouble if the same thing can be done in R. The SAGA and GRASS examples are very good in this respect, as both of them show something that cannot be easily reproduced with R alone.\r\n- [ ] 1.9 [JM] Figure 10.2 – What do the line colors mean?\r\n\r\nChapter 11:\r\n\r\n- [ ] 1.10 [JN] Section 11.2 – Same comment as previously given for the earlier chapters: it may be confusing to deal with both 'Spatial' and 'sf' classes. This situation is inevitable in the R ecosystem at the moment, but needs to be given more emphasis in the book. For example, why does 'crop' work only with 'Spatial' layers, while 'mask' works with 'sf' contrariwise to its documented types of acceptable arguments? (Using 'showMethods(\"mask\")' I now see that 'mask' indeed has an 'sf' method, but it is undocumented). I would actually expect 'mask' to be the less flexible function, given it only needs the rectangular extent. In any case, this situation needs a few words in my opinion. Additionally, the authors may consider adding a comprehensive table specifying which 'raster' functions currently work with 'sf' and which ones require the conversion to 'Spatial'.\r\n- [ ] 1.11 [JN] Section 11.3 – \"which contain a sample of 30 locations with the Zion National Park\" should be \"which contain a sample of 30 locations within the Zion National Park\"\r\n- [ ] 1.12 [JN] Section 11.3 – When creating the 'zion_transect' variable, a CRS should be set to highlight the fact that both layers need to correspond when extracting data from a raster\r\n- [ ] 1.13 [JN] Section 11.3 – The code section preceding Figure 11.3 is not working for me - transect_df$dist = c(0, cumsum(geosphere::distGeo(transect_coords))) Error in .pointsToMatrix(p2) : argument \"p2\" is missing, with no default Anyway, the following expression perhaps can be an alternative - transect_df$dist = geosphere::distGeo(transect_coords, transect_coords[1,])\r\n- [ ] 1.14 [JN] transect_df$dist = geosphere::distGeo(transect_coords, transect_coords[1,]) Section 11.3 – The authors can mention that the necessity for returning a 'list' in raster extraction from lines/polygons is that the features overlap with variable amounts to raster cells, whereas each point always corresponds to a single raster cell (points exactly on the border between two cells are assigned to one cell or the other).\r\n- [ ] 1.15 [JN] Section 11.3 – The authors can mention that when one needs to get a vector of summarized line/polygon values per feature, using just one function, then the 'fun' parameter of function 'extract' can be a quicker way than extracting all values and using 'tidyverse'.\r\n- [ ] 1.16 [JN] Section 11.4 – To complete the discussion of choosing rasterization resolution, it is worth mentioning that sometimes the template resolution is dictated in advance: for example when the set of layers to be aligned already contains at least one raster, or when interpolating into a required area of interest with a predetermined resolution, and so on.\r\n- [ ] 1.17 [JN] Section 11.4 – \"By default fun = 'last is used\" should be \"By default fun = 'last' is used\"\r\n- [ ] 1.18 [JM] Section 11.5 – Text says \"Contour lines can be created with the raster function rasterToContour() , which is itself a wrapper around contourLines() , as demonstrated below:\", but there is no code example for function 'rasterToContour'. Perhaps an example of 'rasterToContour' may be given, followed by 'plot', rather than just using 'contour' which calculates the contour lines \"behind the scenes\".\r\n- [ ] 1.19 [JM] Section 11.5, Figure 11.8 – The best contour labeling algorithm is probably in the 'contour' function, so I would use it in the visual example instead of 'tmap'. General comment: a nice- looking plot such as Figure 11.8 with no code is perhaps less useful than a simpler plot given with reproducible code. For example, the basic plot produced by 'plot(r); contour(r, add=TRUE)' may be a more tangible example.\r\n- [ ] 1.20 [JN] Section 11.5, Figure 11.8 – Using 'dissolve=TRUE' in 'rasterToPolygons' gives the same \"Aggregated polygons\" result shown in the figure, but with shorter code.\r\n\r\nChapter 13:\r\n\r\n- [ ] 1.21 [JM] Section 13.2 – To make the landslide dataset \"balanced\", in the given example the non- landslide points are sampled to give similar positive and negative sample sizes (175 vs. 175). Intuitively this seems wrong: the sample should either be completely random (e.g. sampling random points inside the study area, then checking them for landslides) or comprising the entire population (completely mapped study area). Why not divide the study area into a grid (i.e. raster), where samples are grid cells and have the response variable specify whether the cell contains (at least one) landslide, assuming complete mapping? In other words – it is unclear to me what is the rationale behind keeping \"balance\" between sample size of the two categories, at the expense of randomness. Few years ago I did a study on forest mortality and made the above-mentioned decision. The dataset was similar in structure: there are tree mortality locations scattered around the forest. Similarly to the landslides example, there were quite few mortality sites and a lot of non- mortality sites. However, I thought that instead of making the data balanced, the entire range of conditions in the forest needs to be characterized. So I ended up dividing the forest into a regular grid, and having each grid cell sampled as TRUE/FALSE, i.e. whether the grid cell had at least one mortality location inside. The data indeed were unbalanced, with just ~3% of cells being TRUE. (The data then went to a Mixed-Effects GLM considering spatial autocorrelation). Similarly, in presence/absence modelling of species distributions: I would think that entire valid dispersion range (e.g. a continent) needs to be considered as the \"sample\", with all cells given TRUE/FALSE values according to presence/absence of the species in that cell, rather than choose N observed locations and N absent locations. I believe that spatial logistic regression models (e.g. package 'spatstat') use a similar rationale: the spatial window is considered completely mapped, then the point pattern of observed events is modeled as function of background (e.g. raster) predictor also available for the entire area. Admittedly, I have no theoretical justification, it just intuitively seems more correct. If I'm missing something and the authors have any comments on this I will be very happy to learn.\r\n- [ ] 1.22 [JM] Section 13.5.1 – In the first code section, the expression 'coords = lsl[, c(\"x\", \"y\")]' is duplicated.\r\n- [ ] 1.23 [JM] Could be helpful to comment on model interpretation in GLM vs. SVM. For example, how can we interpret the coefficients in GLM? How can we calculate variable importance in machine learning methods, such as SVM?\r\n\r\nAdditional questions:\r\n\r\n- [ ] 1.24 [RL] The idea of an \"Algorithms\" chapter is great. From the chapter draft it seems the example (centroid) is relatively complex though. Also seems like part of the solution will be using an external package 'decido'. I would suggest replacing or supplementing this example with a simpler one, where the solution only requires functions covered elsewhere in the book, such as functions from the 'sf' package only. Just one of many possible suggestions – a function that accepts a polygonal layer and returns a new line layer containing separate line segments (as separate features) along with the azimuth where the exterior of each line segment is facing. This can be useful in urban planning, e.g. to understand how building facades are oriented. Such a function needs several steps: first splitting the polygons to line segments, then extracting each segment coordinates (x1, y1, x2, y2), and finally applying some trigonometry and math to find the azimuth. This does not require anything outside of 'sf' plus base R. Here is an illustration of a sample layer and the resulting azimuth classification - [FIGURE IN THE PDF]\r\n"}}
{"actor":{"display_login":"Robinlovelace"}}