Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proofreading c5 #1128

Merged
merged 2 commits into from
Sep 28, 2024
Merged

Proofreading c5 #1128

merged 2 commits into from
Sep 28, 2024

Conversation

jannes-m
Copy link
Collaborator

No description provided.

@jannes-m jannes-m mentioned this pull request Sep 27, 2024
19 tasks
Copy link
Collaborator

@Robinlovelace Robinlovelace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some definite fixes in there. Will merge.

Some questions to revisit though cc @Nowosad see comments and let's do a last sweep with global search/replace for consistency after all chapters reviewed individually.

@@ -20,7 +20,7 @@ library(spDataLarge)

So far the book has explained the structure of geographic datasets (Chapter \@ref(spatial-class)), and how to manipulate them based on their non-geographic attributes (Chapter \@ref(attr)) and spatial relations (Chapter \@ref(spatial-operations)).
This chapter focuses on manipulating the geographic elements of spatial objects, for example by creating buffers, simplifying and converting vector geometries, and aggregating and resampling raster data.
After reading it --- and attempting the exercises at the end --- you should understand and have control over the geometry column in `sf` objects and the extent and geographic location of pixels represented in rasters in relation to other geographic objects.
After reading it --- and attempting the Exercises at the end --- you should understand and have control over the geometry column in `sf` objects and the extent and geographic location of pixels represented in rasters in relation to other geographic objects.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really? No objections, the capitalisation highlights them, but not sure it's necessary. No biggy either way, we should check to see if this applied consistently.

@@ -51,7 +51,7 @@ Another reason for simplifying objects is to reduce the amount of memory, disk s
it may be wise to simplify complex geometries before publishing them as interactive maps.
The **sf** package provides `st_simplify()`, which uses the Douglas-Peucker algorithm to reduce the vertex count.
`st_simplify()` uses the `dTolerance` to control the level of generalization in map units [see @douglas_algorithms_1973 for details].
Figure \@ref(fig:seine-simp) illustrates simplification of a `LINESTRING` geometry representing the river Seine and tributaries.
Figure \@ref(fig:seine-simp) illustrates simplification of a `LINESTRING` geometry representing the River Seine and tributaries.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -89,8 +89,8 @@ This means the 'topology' is lost, resulting in overlapping and 'holey' areal un
By default it uses the Visvalingam algorithm, which overcomes some limitations of the Douglas-Peucker algorithm [@visvalingam_line_1993].
<!-- https://bost.ocks.org/mike/simplify/ -->
The following code chunk uses this function to simplify `us_states`.
The result has only 1% of the vertices of the input (set using the argument `keep`) but its number of objects remains intact because we set `keep_shapes = TRUE`:^[
Simplification of multipolygon objects can remove small internal polygons, even if the `keep_shapes` argument is set to TRUE. To prevent this, you need to set `explode = TRUE`. This option converts all mutlipolygons into separate polygons before its simplification.
The result has only 1% of the vertices of the input (set using the argument `keep`), but its number of objects remains intact because we set `keep_shapes = TRUE`:^[
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -300,15 +300,15 @@ rotation = function(a){
}
```

The `rotation` function accepts one argument `a` - a rotation angle in degrees.
The `rotation` function accepts one argument `a` --- a rotation angle in degrees.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

plot(b, border = "grey")
plot(x_and_y, col = "lightgrey", border = "grey", add = TRUE) # intersecting area
plot(b, border = "gray")
plot(x_and_y, col = "lightgray", border = "gray", add = TRUE) # intersecting area
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought there was an issue but testing suggests it's OK:

plot(1:9, col = "gray")

Created on 2024-09-28 with reprex v2.1.0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we now use "gray" throughout?

resolution = c("(30.85, 30.85)", "(154.25, 154.25)"),
dimensions = c("117 x 117", "24 x 24"),
extent = c("794599.1, 798208.6, 8931775, 8935384", "794599.1, 798301.1, 8931682, 8935384")
Object = c("dem", "dem_agg"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -118,8 +118,8 @@ arrange(us_states_bor, -borders)
```

E7. Read the srtm.tif file into R (`srtm = rast(system.file("raster/srtm.tif", package = "spDataLarge"))`).
This raster has a resolution of 0.00083 by 0.00083 degrees.
Change its resolution to 0.01 by 0.01 degrees using all of the method available in the **terra** package.
This raster has a resolution of 0.00083 * 0.00083 degrees.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really? cc @Nowosad

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the previous version

@Robinlovelace Robinlovelace merged commit c8635ca into main Sep 28, 2024
1 of 3 checks passed
@Robinlovelace Robinlovelace deleted the proof_c5 branch September 28, 2024 02:37
github-actions bot pushed a commit that referenced this pull request Sep 28, 2024
* add first set of reviewer suggestions

* add second set of changes

---------

Co-authored-by: jannes <jannes@cynkra.com> c8635ca
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants