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

Code Review Request #1

Open
rhirotacouncil opened this issue May 22, 2023 · 1 comment
Open

Code Review Request #1

rhirotacouncil opened this issue May 22, 2023 · 1 comment
Assignees

Comments

@rhirotacouncil
Copy link
Contributor

Code review request for Storefronts_ct_map.Rmd and Storefronts_cd_map.Rmd.

Some things:

  • Storefronts_cd_map.Rmd needs to be run first.
  • The census tract info was initially formatted differently between the census and the vacancy data set. The updated formatting/merged shapefile definitely needs to be checked to make sure the tracts line up correctly. I left some notes on the .Rmd about a few potential issues.

Let me know if anything is confusing!

@romartinez-nycc
Copy link
Contributor

Reorganized the repo to follow the project template structure.

  • As a result, your .gitignore additions were removed. I think you can add it back on your end only and don't commit & push it back into the gitignore file. It's also fine to commit the files as well.
    visuals/vacant_cd_2022_files/leaflet-1.3.1/images/layers.png
    visuals/vacant_cd_2022_files/leaflet-1.3.1/images/marker-icon-2x.png
    visuals/vacant_cd_2022_files/leaflet-1.3.1/images/marker-icon.png
    visuals/vacant_cd_2022_files/leaflet-1.3.1/images/marker-shadow.png
    visuals/vacant_cd_2022_files/leaflet-1.3.1/leaflet.css
    visuals/vacant_cd_2022_files/leaflet-1.3.1/leaflet.js
    visuals/vacant_cd_2022_files/leaflet-binding-2.1.2/leaflet.js
    visuals/vacant_cd_2022_files/leafletfix-1.0.0/leafletfix.css
    visuals/vacant_cd_2022_files/map-widget-style-1.0.0/map-widget-style-bindings.js
    visuals/vacant_cd_2022_files/proj4-2.6.2/proj4.min.js
    visuals/vacant_cd_2022_files/Proj4Leaflet-1.0.1/proj4leaflet.js
    visuals/vacant_cd_2022_files/rstudio_leaflet-1.3.1/images/1px.png
    visuals/vacant_cd_2022_files/rstudio_leaflet-1.3.1/rstudio_leaflet.css
    visuals/vacant_cd_2022_files/htmlwidgets-1.6.2/htmlwidgets.js
    visuals/vacant_cd_2022_files/jquery-1.12.4/jquery.min.js
    visuals/vacant_cd_2022_files/leaflet-1.3.1/images/layers-2x.png
    visuals/.DS_Store
  • You can move your libraries into the 00_load_depencies.R file.
    library(dplyr)
    library(janitor)
    library(ggplot2)
    library(ggthemes)
    library(stringr)
    library(zoo)
    library(data.table)
    library(sf)
    library(sp)
    library(tidyr)
    library(leaflet)
    library(tibble)
    library(htmltools)
    library(leaflet.extras)
    library(RSocrata)

Storefront_cd_map.rmd

No issues in running the code, I was able to run it all the way through 🎉

Code efficiency suggestions

Vroom::vroom() is a bit faster if you'd like to use it.

leased_not_leased_2022 <- read.csv("https://data.cityofnewyork.us/resource/92iy-9c3n.csv?$limit=999999999") %>% clean_names()

Code documentation

It's helpful to comment on how you are validating your bin choice for the variable being mapped. (There is a small typo, the column is vacant_2022.)

#plot(density(vacant_cd.shp$vacant_2021))
#
int_nta <- classInt::classIntervals(vacant_cd.shp$vacant_2022, n = 5, style = 'sd')
# pal_cd <- colorBin(
# palette = c("#800000","#a44736","#c37b6c","#ddafa7","#e5cccc"),
# bins = int_nta$brks,
# domain = vacant_cd.shp$vacant_2021,
# na.color = "#E6E6E6",
# reverse = TRUE
# )

- Here is a comment example from Nick: https://github.com/NewYorkCityCouncil/electrifying-transportation/blob/ecb876dc05457d67541c9d4dc10e2c6f836c2c47/code/02_car-ownership_map.R#L16
- Here is an example of how your commented-out code could work:

int_cd <- classInt::classIntervals(vacant_cd.shp$vacant_2022, n = 5, style = 'jenks')

pal_cd <- colorBin(
 palette = nycc_pal("warm")(5),
 bins = int_cd$brks,
 domain = vacant_cd.shp$vacant_2021,
 na.color = "#E6E6E6",
)

Map Improvements

  1. Legend

Above, I used the existing council red-warm palette from the webpage, but open to discussing whether we should change it to the council blue. I don't have a preference. For nycc_pal("warm")(5), we should try to limit our bins to a max of 7, drawing from behavioral science, a legend with more than 7 can be too much for the human brain to take in. For the census tract map, since the tracts are small, we can consider playing with a continuous range of colors (heatmap style) over one that has been divided into bins of discrete colors.

  1. No need to define the following leaflet parameters:
  • min/max zoom (were recently added to theaddCouncilStyle() function thanks to James!)
  • setView
  • white background
  • It's easier to leave the height & width parameters undefined at this stage. We can adjust the size within the WordPress iframe stage for the HTML output or within mapshot for the static export ex.vwidth = 900, vheight = 870.
    m <- leaflet(vacant_cd.shp,
    options = leafletOptions(minZoom = 10), height = 700, width = 700
    ) %>%
    addCouncilStyle(add_dists = TRUE) %>%
    # addPolygons(data=boro, stroke = T, fill=F, color = "#666666", weight = 1) %>%
    addPolygons(weight = 1,
    fillColor = ~pal_cd(vacant_2022),
    color="Black",
    stroke = TRUE,
    fillOpacity = .7,
    popup = paste("Council District: ", vacant_cd.shp$council_district, "</br>","Number Vacant: ", vacant_cd.shp$count, "</br>", "Percent Vacant: ", paste(vacant_cd.shp$vacant_2022,"%", sep=""))) %>%
    addLegend(position ="topleft",
    pal=pal_cd,
    values = vacant_cd.shp$vacant_2022,
    title = "Percent Vacant",
    labFormat = labelFormat(suffix = "%", digits = 1),
    opacity = 1) %>%
    setMapWidgetStyle(list(background= "white")) %>%

You may need to reinstall the councildown package to pull in the latest updates
remotes::install_github("newyorkcitycouncil/councildown")

Map Suggestions

  1. I tend to read in datasets into leaflet, for example, the vacant_cd.shp dataset within the addPolygon or other addLealfet functions over including it in the main leaflet() section. This is most helpful when mapping multiple datasets, for example, if you also wanted to add boro polygons, it may lead to fewer code issues.
    m <- leaflet(vacant_cd.shp,
  2. Running the map gave me the chance to play around with Anne's new councildown map functions like addLegend_decreasing() and the highlight_dists = parameter within addCouncilStyle(). I will share the jpeg of how that ended up looking.

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

No branches or pull requests

3 participants