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.

Sign up for GitHub

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

Jump to bottom

Pluto zoning tweaks #977

Merged
merged 2 commits into from
Jul 24, 2024
Merged

Pluto zoning tweaks #977

merged 2 commits into from
Jul 24, 2024

Conversation

fvankrieken
Copy link
Contributor

@fvankrieken fvankrieken commented Jul 5, 2024 •
edited
Loading

A couple things were discovered in #972 for ztl that are the same for pluto

  1. Bug in special districts - sorting wrong order. This changes about 60 lots - outside of the lots that have their ordering manually set for the specific combo of special districts, there are luckily very few that are in multiple
  2. Use grouping logic in commercial overlay just like we do for zoning districts. This ends up only affecting a few rows as well

Running build here to compare to nightly_qa. Differences are

  • 40 sp dist flips (expected). I need to confirm with GIS about these (and ordering in ztl)
  • 12 commercial dist changes (expected) - due to grouping by commercial overlay (so if there are two smaller C-2 overlays, that's captured)
  • 1 splitzone change (due to a commercial overlay change)
  • 25 lottype changes - unexpected. See PLUTO lottype is not deterministic #1012

@fvankrieken fvankrieken force-pushed the fvk-pluto branch 2 times, most recently from 8a52cd6 to 9caa224 Compare July 12, 2024 19:20
@fvankrieken fvankrieken mentioned this pull request Jul 16, 2024
Open
@damonmcc damonmcc mentioned this pull request Jul 16, 2024
Closed
38 tasks
@fvankrieken fvankrieken marked this pull request as ready for review July 17, 2024 14:06
@fvankrieken
Copy link
Contributor Author

Got go-ahead from GIS on these changes

@sf-dcp sf-dcp self-requested a review July 17, 2024 17:31
sf-dcp
sf-dcp reviewed Jul 17, 2024
View reviewed changes
products/pluto/pluto_build/02_build.sh
Comment on lines -77 to -78
run_sql_file sql/zoning_correctdups.sql
run_sql_file sql/zoning_correctgaps.sql
Copy link
Contributor

Choose a reason for hiding this comment

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

is this change related to the new group logic mentioned in the second bullet point?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup - this deduplication happens via the GROUP BY clause now

sf-dcp
sf-dcp approved these changes Jul 17, 2024
View reviewed changes
Copy link
Contributor

@sf-dcp sf-dcp left a comment

Choose a reason for hiding this comment

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

LGTM! Curious, did you end up using the pandas test util for comparing data frames to identify the data changes?

@fvankrieken
Copy link
Contributor Author

No, I used Pluto's QA queries. So

  • checking total row counts
  • checking total number of changes per column (changes identified by bbl)
  • investigating changed rows for rows with changes in columns found above (1 column at a time)

@sf-dcp
Copy link
Contributor

sf-dcp commented Jul 22, 2024

merge?

@fvankrieken
Copy link
Contributor Author

Going over one more time with Casey today (who was out previously), so figured I'd hold off. Will get this merged as-is or with tweaks today!

@fvankrieken fvankrieken merged commit cadc41a into main Jul 24, 2024
26 checks passed
@fvankrieken fvankrieken deleted the fvk-pluto branch July 24, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sf-dcp sf-dcp sf-dcp approved these changes

Assignees

@damonmcc damonmcc

@sf-dcp sf-dcp

Labels
None yet
Projects
Data Engineering
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fvankrieken @sf-dcp @damonmcc