Skip to content

Feedback - #1

Open
github-classroom[bot] wants to merge 16 commits into
feedbackfrom
main
Open

github-classroom[bot] wants to merge 16 commits into
feedbackfrom
main

Conversation

@github-classroom

@github-classroom github-classroom Bot commented Apr 24, 2026 •

Copy link
Copy Markdown

👋! GitHub Classroom created this pull request as a place for your teacher to leave feedback on your work. It will update automatically. Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.
In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.
Click the Files changed or Commits tab to see all of the changes pushed to the default branch since the assignment started. Your teacher can see this too.

Notes for teachers

Use this PR to leave feedback. Here are some tips:

  • Click the Files changed tab to see all of the changes pushed to the default branch since the assignment started. To leave comments on specific lines of code, put your cursor over a line of code and click the blue + (plus sign). To learn more about comments, read “Commenting on a pull request”.
  • Click the Commits tab to see the commits pushed to the default branch. Click a commit to see specific changes.
  • If you turned on autograding, then click the Checks tab to see the results.
  • This page is an overview. It shows commits, line comments, and general comments. You can leave a general comment below.
    For more information about this pull request, read “Leaving assignment feedback in GitHub”.

Subscribed: @hassanmiran1 @DannyinDelft

@cforgaci cforgaci left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@Applied-Spatial-Analytics/groupc thanks for updating the report. Which parts of the code do you need feedback on?

@@ -0,0 +1,279 @@
---
title: "Green Space Landscape Metrics"
author: "Hassan, Danny and Akhil"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use proper formatting with author details: https://quarto.org/docs/journals/authors.html

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@Applied-Spatial-Analytics/groupc, nice that you use R to carry out the steps of your analysis and that you include the code in code chunks in the report. Remember to embed the code in a proper report structure (intro, methods, results, discussion, references), with text and figures.

@cforgaci cforgaci left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@Applied-Spatial-Analytics/groupc thanks for updating the report. Which part of the code do you want feedback on?

library(landscapemetrics)

green <- rast(
"D:/ARFW0501/newest/Data/green_binary_local_v2.tif")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Absolute path to data — change to relative

Comment on lines +28 to +48
# Step 2. Test one grid cell

```{r}
test_cell <- vect(grid[500, ])

green_test <- crop(green, test_cell)

green_test <- mask(green_test, test_cell)

plot(green_test)

```

# Step 3. Calculate landscape metrics one grid cell

```{r}
lsm_p_area(green_test)
lsm_p_gyrate(green_test)
lsm_p_contig(green_test)
lsm_p_enn(green_test)
lsm_p_PROX(green_test)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remove from final version

Comment thread sections/problem_statement.qmd Outdated
# Problem statement

## Background
Urban flooding is becoming a growing issue in many cities, especially as heavy rainfall events become more intense and urban areas continue to expand. In dense urban environments, large amounts of impervious surface can reduce infiltration and increase surface runoff. Green spaces can help reduce this pressure by storing water, slowing runoff, and allowing more water to infiltrate into the ground.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This seems to be a very strong statement, a reference might be good to backup your claim.

```{r}

# Step 1: Snap flood to green raster grid exactly
flood_aligned <- resample(flood, green, method = "near")

@a-kurski a-kurski Jun 16, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why method = "near"? both are continuous phenomena

Comment thread sections/problem_statement.qmd Outdated
## Background
Urban flooding is becoming a growing issue in many cities, especially as heavy rainfall events become more intense and urban areas continue to expand. In dense urban environments, large amounts of impervious surface can reduce infiltration and increase surface runoff. Green spaces can help reduce this pressure by storing water, slowing runoff, and allowing more water to infiltrate into the ground.

However, the role of green space is not only about how much green area is present. Its spatial configuration may also matter. A city district with one large and connected green area may respond differently to rainfall than a district with the same amount of green space divided into many small and isolated patches. This makes it important to look at landscape metrics such as patch size, fragmentation, aggregation, and connectivity.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The first 2 sentences don't really make sense to me. While I could guess what you mean and what direction you want to go, I think rephrasing the first 2 sentences will really enhance the user understanding of the report.

Comment thread docs/sections/preliminary.html Outdated
<p>The first step of the project was to define comparable study areas for Delft and Xi’an. Since the project compares two very different urban contexts, it was important to use the same spatial logic for both cities. Instead of using the full administrative boundaries, a 10 km × 10 km study area was selected for each city. Each study area was then divided into 100 m × 100 m grid cells.</p>
<p>This grid-based approach makes the two cities easier to compare. It creates a common spatial unit, so that green-space metrics and flood-related values can be calculated in the same way for both Delft and Xi’an. This also avoids comparing only city-wide averages, which would hide many local differences inside the urban fabric.</p>
<p>For Delft, the selected area includes the city itself and parts of the surrounding urban and rural landscape. This is useful because Delft is not isolated from its surroundings. Its green and water structures continue beyond the municipal boundary. For Xi’an, the selected area focuses on a central urban section where built-up land, green patches, and flood-related surface water change can be analysed together. In both cases, the study area was chosen to capture local spatial patterns rather than the entire city.</p>
</section>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here you can insert relevant city maps (or satellite imagery) to help the reader understand the spatial context of your study area.

Comment thread docs/sections/preliminary.html Outdated
<p>The built environment is important in this project because impervious surfaces influence how water behaves during heavy rainfall. Roads, roofs, pavements, and other hard surfaces reduce infiltration and can increase surface runoff. Because of this, green-space configuration needs to be understood in relation to the surrounding urban fabric.</p>
<p>The first land-cover exploration showed that data quality and resolution were major issues. For Delft, broad land-cover datasets such as ESA WorldCover were tested, but they were not detailed enough for the scale of the project. More local Dutch data sources were therefore explored. Because the 10 km × 10 km study area extended beyond the municipality of Delft, surrounding municipalities also had to be included and merged. The final Delft green-space layer was converted into a binary raster, where green areas were represented as 1 and non-green areas as 0.</p>
<p>For Xi’an, the first land-cover datasets also caused problems. Some products were too coarse, while others did not clearly capture smaller green spaces inside the urban area. This was a problem because the project depends on recognising the spatial configuration of green patches. If small parks, street greenery, or fragmented patches are missing, the landscape metrics become less meaningful.</p>
<p>To improve this, a more detailed green-space dataset was used for Xi’an. The final Xi’an green layer was based on the UGS-1m dataset and was also converted into a binary raster. This made it possible to calculate the same type of landscape metrics for Xi’an as for Delft.</p>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please make sure you clearly mention the dataset for Xi’an.

Comment thread docs/sections/preliminary.html Outdated
<p>The project focuses on pluvial flooding. This means flooding caused by heavy rainfall and local surface water accumulation, rather than flooding from rivers or the sea. Pluvial flooding is strongly connected to urban land cover. In areas with many impervious surfaces, rainfall cannot easily infiltrate into the ground. This can increase runoff and cause water to collect on streets, squares, and other low-lying parts of the city.</p>
<p>Green spaces can help reduce this problem because they are more permeable than built-up surfaces. They can support infiltration, slow down runoff, temporarily store water, and reduce pressure on drainage systems. However, the effect of green space may not only depend on the total amount of green area. Its spatial configuration may also matter. A large connected park may influence runoff differently than many small isolated green patches, even if the total green area is similar.</p>
<p>For this reason, the project does not only measure green coverage. It also uses landscape metrics that describe the spatial pattern of green areas, including patch size, fragmentation, aggregation, and connectivity.</p>
<p>Flood-related conditions were first explored using different available datasets. For Delft, Dutch climate data and water-depth layers for intense rainfall were tested. However, an equivalent pluvial flood-depth layer was not available for Xi’an. To keep the method more comparable between the two cities, the analysis moved toward using FWEI, the Flood Water Extraction Index.</p>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please add a reference for the FWEI index here and provide a brief explanation of the concept and methodology behind it. Introduce what the index measures, its main components/indicators, and how it is calculated.

@cforgaci cforgaci left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@Applied-Spatial-Analytics/groupc I left a few comments for you to consider before the submission of the report. Note that I also enabled GitHub Pages for you: https://applied-spatial-analytics.github.io/create-your-report-groupc/

Comment thread index.qmd Outdated

This project investigates the relationship between green space configuration and pluvial flooding in Delft and Xi’an. The main focus is not only the amount of green space, but also how green spaces are spatially arranged.

Because comparable pluvial flood risk maps were not available for both cities, FWEI derived surface water change is used as a proxy.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The reader does not know what FWEI is. Please add a reference.

Comment thread index.qmd Outdated
Comment on lines +9 to +11
Because comparable pluvial flood risk maps were not available for both cities, FWEI derived surface water change is used as a proxy.

Both cities are analysed using a grid based analysis approach. Green space metrics, flood related indicators, and DEM variables are calculated for each grid cell, after which the results are compared using correlation analysis, metric selection, and combined typology construction.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These lines belong to the Methods section

Comment thread sections/methods.qmd

The workflow was implemented mainly in R, using the packages `terra`, `sf`, `landscapemetrics`, `dplyr`, and `ggplot2`. QGIS was used for data preparation, visual checks, and the creation of study area context maps.

![Grid-based methodology used for comparing Delft and Xi’an.](../figures/preliminary/grid_methodology.png){#fig-grid-methodology width=100%}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't see the typology in this diagram.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also, it would be great if the following sections (3.1, 3.2 ...) are name consistetly with the steps listed in this figure.

Comment thread sections/methods.qmd Outdated
| Redundancy tables | Show which metrics were removed or kept. |
| Combined typology maps | Show the final green+flood+DEM typology in each city. |
| Typology summary tables | Describe the average characteristics of each type. |
| Problematic typology candidates | Identify typology types for later interpretation and possible nature-based solution discussion. |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What are problematic typology candidates? Are these outliers, invalid candidates which cannot be meaningfully related to the clustering results?

Comment thread sections/metrics.qmd

`elevation_mean` shows the average elevation per grid cell.

![Delft mean elevation.](../figures/results/delft_elevation_mean_context.png){width=49%}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The link seems to be broken here

Comment thread sections/discussion.qmd Outdated

The workflow is partly reproducible. After the input data are prepared, the R scripts can be used to produce the grid cell metrics, metric selection outputs, correlation tables, maps, and final typologies. The main scripts and report files are included in the GitHub repository, while the necessary larger datasets are stored in the shared SURFdrive folder.

The repository is organised so that the report can be rendered with Quarto. The main outputs are stored in `data/results` and `figures/results`. The most important R scripts are used for calculating green space metrics, generating maps and plots, selecting metrics, and producing the final combined typologies. A README file describes the repository structure, the main scripts, the data folders, and the steps needed to rerun or render the analysis.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The README file does not seem to provide this information.


## Background

Pluvial flooding is a relevant issue in urban areas. It occurs when intense rainfall creates overland flow or surface ponding before water can enter drainage systems, canals, rivers, or other water bodies [@prokic2019]. In cities, impervious surfaces such as roads, roofs, and pavements can reduce infiltration and increase the amount of water remaining on the surface.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This paragraph is a bit clumsy. State why this is important, not just that it is important, give us a picture of how it can impact urban areas.

@@ -0,0 +1,44 @@
# Problem statement

## Background

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Overall well constructed

Comment thread sections/problem_statement.qmd Outdated

Many studies discuss green infrastructure and nature based solutions in relation to stormwater management and flood risk [@sarabi2022]. However, comparing this relationship between different cities is difficult because cities differ in size, climate, data availability, spatial resolution, and land cover structure.

Delft and Xi’an provide two different urban contexts. Delft is a smaller Dutch city in a low lying landscape with a dense water network. Xi’an is a much larger Chinese city with a different urban structure and spatial scale. To make comparison possible, this project uses equal sized study areas and a shared grid based method.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

different urban contexts and maybe climates ?

Comment thread sections/methods.qmd Outdated

![Grid-based methodology used for comparing Delft and Xi’an.](../figures/preliminary/grid_methodology.png){#fig-grid-methodology width=100%}

The figure above shows the basic idea of the method. A common grid is used so that different spatial datasets can be converted into comparable values per cell. This makes it possible to compare green space patterns, FWEI derived surface water change, and DEM variables using the same spatial unit.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It is a bit colloquial (the first sentence is redundant). there is a bit of repetition with the first paragraph of this section regarding the use of the grid for comparison purposes, I suggest you tighten it by avoiding repeating the argument in this paragraph.
moreover, every time you introduce a acronym you should spell it out, for example: "the United Nations (UN) do ..." and then I can use UN in the text without any problem.

Comment thread sections/methods.qmd Outdated
Comment on lines +32 to +36
Both cities were analysed using the same 100 m by 100 m grid structure. This grid was used as the common spatial unit for the analysis. Instead of comparing whole city averages, each grid cell received its own green space, flood related, and DEM values.

This approach was useful because the input layers had different resolutions and formats. Some layers were raster datasets, while others were vector layers or derived outputs. By summarising everything per grid cell, the analysis created one comparable table for Delft and one for Xi’an.

Each city had 10,000 grid cells. The same grid size was used for both cities, so differences between Delft and Xi’an could be interpreted more consistently.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Beware of the aforementioned repetition (see my comment on line R11)

Comment thread sections/metrics.qmd Outdated
Comment on lines +3 to +5
This chapter explains which metrics were calculated and how the final metric set was selected. At first, a larger set of green-space, FWEI derived flood, distance to green, and DEM metrics was produced for both Delft and Xi’an. However, not all of these metrics were used in the later analysis, because some of them described almost the same spatial pattern.

The aim of this chapter is therefore to show the full metric set, explain the redundancy check, and identify the metrics that were kept for the combined typology analysis.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A lot of repetition again, tighten up your text, this could be a single paragraph.

Comment thread sections/typology.qmd Outdated
@@ -0,0 +1,99 @@
# Typology construction

This chapter explains how the combined typologies were constructed and how the four final types were defined. The main purpose of this chapter is to show how the grid cells were grouped and what the resulting typology classes represent.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the two sentences are repeating each other.

Comment thread sections/discussion.qmd Outdated

AI tools were used during the project as support for writing, structuring the report, checking R code and combining scripts, improving explanations, and debugging parts of the workflow. AI was also used to help translate technical results into clearer report text. The analysis itself was still based on the datasets, R scripts, maps, and outputs produced during the project. AI was not used as a replacement for the spatial analysis, but as a support tool during coding and phrasing.

## Future improvements

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this section would work well if combined with the limitation, avoiding unnecessary repetitions

Comment thread sections/discussion.qmd Outdated

The workflow is not fully automatic from the very beginning. Some preprocessing steps are necessary, including finding suitable datasets, clipping the study areas, checking coordinate reference systems, preparing input layers, and deciding which datasets were comparable between Delft and Xi’an. These manual decisions are important because they influence the final outputs.

AI tools were used during the project as support for writing, structuring the report, checking R code and combining scripts, improving explanations, and debugging parts of the workflow. AI was also used to help translate technical results into clearer report text. The analysis itself was still based on the datasets, R scripts, maps, and outputs produced during the project. AI was not used as a replacement for the spatial analysis, but as a support tool during coding and phrasing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Which tools ? Be specific please.

@UrbanMaple

Copy link
Copy Markdown

Overall my main points for improvement would be:

  1. to have a human eye re-reading the text and removing/tightening the text to avoid so many repetitions, it really dilutes your message.
  2. to introduce the acronyms (a reference when you state what it does would be a nice addition too).

@yehanwu yehanwu left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well done on the typology construction results and discussion. The process of choosing the metrics is also well documented.

Comment thread sections/typology.qmd
)

knitr::kable(final_typology_small, caption = "Summary of the final shared green+flood+DEM typology.")
```

@yehanwu yehanwu Jun 25, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It would be helpful to name and characterise each typology. Also I see this table shown in section 6.1. Consider removing one of them.

This branch was successfully deployed

1 active deployment
github-pages — 1e388551 Deployed Jun 26, 2026 by hassanmiran1 via deploy #5
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.

8 participants