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

Add equity map #87

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Add equity map #87

wants to merge 3 commits into from

Conversation

mstone121
Copy link
Contributor

Overview

This PR adds a basic version of the the equity map.

The equity data shapefile is large, >300mb, and can't be served to the frontend in one piece. In order to reduce the size of the data, I performed the following steps:

  • Filtered out shapes that were not disadvantaged (SN_C == 0)
  • Dissolved the remaining (SN_C == 1) shapes into one shape
  • Simplified the shape by retaining 1% of the data points (Douglas-Peucker method)

Then I used dirty-reproject to project the map with albersUsa. (For some reason this created bounding-box shaped rings around each element. The best way I could figure out to delete these was to do a find and replace in the JSON file, though this was time consuming and error-prone.)

The map in it's current state is not done and doesn't match the design exactly, but the amount of data displayed (and the limitation of it being just one giant feature) also differentiates it from the design. I figured we could start here and re-evaluate display of this data. A few things to consider are:

  • Does the diagnol lines pattern still work? I made the lines thinner and closer together so smaller details of the shape wouldn't get lost.
  • How exactly do we want to display this data with the spending data? Perhaps we could make the disadvantaged communities a toggle-able layer.

Closes #67

Demo

Screenshot 2023-03-22 at 1 30 25 PM

Testing Instructions

Checklist

  • fixup! commits have been squashed
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines
  • README.md updated if necessary to reflect the changes
  • CI passes after rebase

@github-actions
Copy link

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 1 0 0.02s
✅ BASH shfmt 11 0 0.01s
✅ JAVASCRIPT prettier 42 0 5.37s
✅ JSON eslint-plugin-jsonc 3 0 0.78s
✅ JSON jsonlint 3 0 0.14s
✅ SPELL misspell 87 0 0.1s
✅ TERRAFORM terraform-fmt 5 0 0.15s
✅ YAML yamllint 5 0 0.33s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

Copy link
Contributor

@rachelekm rachelekm left a comment

Choose a reason for hiding this comment

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

Great job wrangling all this data, for how much is there I think it looks really good and renders quickly. I also didn't know about dirty-reproject which is pretty cool. I left two comments, but they're minor and this successfully gets a basic equity map up so we can move forward discussing how best to display this data. FWIW I think the diagonal lines work but breaks down by the midwest/southeast—but I could also see this data being compelling as a toggle-able layer over the per-capita map and maybe that diagonal pattern will be most visible over colored states this way? Interested to see where this goes!

}, [map, disadvantagedCommunitiesData]);

useEffect(() => {
var svg = map
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var svg = map
const svg = map


useEffect(() => {
import('../../data/disadvantagedCommunities.json').then(data => {
setDisadvantagedCommunitiesData(data as unknown as GeoJSON.GeoJSON);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this useEffect is getting called every time we toggle between the per capita map and then back to the equity map. After the initial toggle to the equity map I would think we wouldn't need to re-render, would useMemo be more appropriate here? Also not sure how relevant this is if we're eventually refactoring this as a toggle-able layer.

@mstone121
Copy link
Contributor Author

Closed since we're proceeding with a different equity map. See #100 for replacement.

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.

Add equity map
2 participants