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 new permits view #597

Merged

Conversation

jeancochrane
Copy link
Contributor

This PR defines a new view default.vw_pin_permit that pulls data from iasworld.permit, cleans it up a bit, and documents it. The goal is to produce a view on top of iasworld.permit that is easier to use for modeling and analysis.

Note that I still have a number of open questions about the meaning of some of these fields, so the documentation is not complete. I've done my best to determine the most important fields in the absence of complete documentation, but I would appreciate if you compared the columns I decided to pull out to the columns that are visible in iasWorld and let me know if you think I've missed anything important.

@jeancochrane jeancochrane linked an issue Sep 12, 2024 that may be closed by this pull request
Comment on lines 53 to 120
- null: No improvement code assigned
- `110 - NEW CONSTRUCTION - MAJOR IMPRVT`
- `111 - NEW BUILDING`
- `112 - ADDITIONS`
- `113 - DORMERS`
- `114 - OTHER - MAJOR NEW CONSTRUCTION`
- `130 - NEW CONSTRUCTION - MINOR IMPRVMT`
- `131 - SWIMING POOL - TENNIS COURT`
- `132 - DRIVEWAYS - PATIOS - WOOD DECK`
- `133 - FENCING (and Gates)`
- `134 - GARAGE CARPORTS BARNS`
- `135 - OTHER-MINOR NEW CONSTRUCTION`
- `151 - BASEMENT ROOM - REC ROOM`
- `152 - ATTIC ROOM - BATHROOMS`
- `153 - FIREPLACE-CENTRAL AIR CONDITIONING`
- `154 - OTHER - REMODELING`
- `155 - ???`
- `156 - REMODELING - INTERIOR ONLY`
- `157 - REMODELING - EXTERIOR ONLY`
- `171 - MAJOR IMPROVEMENT - WRECK`
- `172 - MINOR IMPROVEMENT - WRECK`
- `191 - MAJOR IMPROVEMENT - BURNOUT`
- `192 - MINOR IMPROVEMENT - BURNOUT`
- `211 - NEW BUILDING`
- `212 - ADDITIONS`
- `213 - OTHER - MAJOR NEW CONSTRUCTION`
- `231 - SWIMMING POOL - TENNIS COURT`
- `232 - DRIVEWAYS - PATIOS`
- `233 - FENCING`
- `234 - GARAGE-BARN-BUTLER BUILDING`
- `235 - OTHER-MINOR NEW CONSTRUCTION`
- `236 - ???`
- `251 - PARTITIONING`
- `252 - DECONV/CONV AMT. LIV. UT.`
- `253 - AIR CONDITIONING - FIREPLACES`
- `254 - BASEMENT-ATTIC ROOM BATH`
- `255 - REPLACE-REPAIR BUIDLING - FACTORY`
- `256 - OTHER REMODELING`
- `257 - REMODELING - INTERIOR ONLY`
- `258 - REMODELING - EXTERIOR ONLY`
- `271 - MAJOR IMPROVEMENT - WRECK`
- `272 - MINOR IMPROVEMENT - WRECK`
- `291 - MAJOR IMPROVEMENT - BURNOUT`
- `292 - MINOR IMPROVEMENT - BURNOUT`
- `300 - RAILROAD PROPERTIES`
- `310 - NEW CONST MAJOR IMPROVEMENT`
- `311 - CARRIER - NEW MAJOR CONSTRUCTION`
- `312 - NON-CARRIER NEW MAJOR CONSTRUCTION`
- `330 - NEW CONSTRUCTION MINOR IMPROVEMENT`
- `331 - CARRIER - REMODELING`
- `332 - NON-CARRIER-REMODELING`
- `350 - REMODELING`
- `351 - CARRIER - REMODELING`
- `352 - NON-CARRIER - REMODELING`
- `370 - WRECKS`
- `371 - MAJOR IMPROVEMENT WRECK`
- `372 - MINOR IMPROVEMENT - WRECK`
- `390 - BURNOUTS`
- `391 - MAJOR IMPROVEMENT - BURNOUT`
- `392 - MINOR IMPROVEMENT - BURNOUT`
- `400 - EXEMPT PROPERTIES`
- `411 - LEASEHOLD`
- `510 - RECHECKS`
- `511 - OCCUPANCY`
- `512 - PRIOR YEAR PARTIAL ASSESSMENT`
- `515 - MANUALLY ISSUED PERMIT`
- `516 - PRIOR YEAR RECHECK`
- `900 - OTHER`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about adding these values as a seed instead of documenting here, so that we could join the seed to the permit view and populate a field with the human-readable value for the improvement code. I figure that's an iterative improvement we can pick up if we decide this field is important to us.

Copy link
Member

Choose a reason for hiding this comment

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

Per our convo the other day, I think we should move the enumerated values of fields to seeds in the ccao DB, then use them for checks against their respective source fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed! To clarify, do you think that's worth doing now, or punting to a later improvement?

Copy link
Member

Choose a reason for hiding this comment

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

We can punt that to a later PR. Let's open an issue to:

  1. Create a unified schema for storing enumerated column values as seeds
  2. Move these existing values to said schema and out of the docs

As a rule of thumb, I'd say anytime a column has more than 5 enumerated values then the values should live in the seed(s), with a pointer from the docs to the seed(s). If <= 5 values, let's keep the enumerated value in the docs and add it to the seed(s).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I opened an issue in #599.

@jeancochrane jeancochrane force-pushed the jeancochrane/239-create-dbt-model-for-permits-view branch from f11c845 to 89341b7 Compare September 12, 2024 19:06
@jeancochrane jeancochrane marked this pull request as ready for review September 12, 2024 19:23
@jeancochrane jeancochrane requested a review from a team as a code owner September 12, 2024 19:23
@jeancochrane jeancochrane requested a review from dfsnow September 12, 2024 19:23
Copy link
Member

@dfsnow dfsnow left a comment

Choose a reason for hiding this comment

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

Great work @jeancochrane. Excited to have this as an asset and as a (potential) open data set.

Comment on lines 53 to 120
- null: No improvement code assigned
- `110 - NEW CONSTRUCTION - MAJOR IMPRVT`
- `111 - NEW BUILDING`
- `112 - ADDITIONS`
- `113 - DORMERS`
- `114 - OTHER - MAJOR NEW CONSTRUCTION`
- `130 - NEW CONSTRUCTION - MINOR IMPRVMT`
- `131 - SWIMING POOL - TENNIS COURT`
- `132 - DRIVEWAYS - PATIOS - WOOD DECK`
- `133 - FENCING (and Gates)`
- `134 - GARAGE CARPORTS BARNS`
- `135 - OTHER-MINOR NEW CONSTRUCTION`
- `151 - BASEMENT ROOM - REC ROOM`
- `152 - ATTIC ROOM - BATHROOMS`
- `153 - FIREPLACE-CENTRAL AIR CONDITIONING`
- `154 - OTHER - REMODELING`
- `155 - ???`
- `156 - REMODELING - INTERIOR ONLY`
- `157 - REMODELING - EXTERIOR ONLY`
- `171 - MAJOR IMPROVEMENT - WRECK`
- `172 - MINOR IMPROVEMENT - WRECK`
- `191 - MAJOR IMPROVEMENT - BURNOUT`
- `192 - MINOR IMPROVEMENT - BURNOUT`
- `211 - NEW BUILDING`
- `212 - ADDITIONS`
- `213 - OTHER - MAJOR NEW CONSTRUCTION`
- `231 - SWIMMING POOL - TENNIS COURT`
- `232 - DRIVEWAYS - PATIOS`
- `233 - FENCING`
- `234 - GARAGE-BARN-BUTLER BUILDING`
- `235 - OTHER-MINOR NEW CONSTRUCTION`
- `236 - ???`
- `251 - PARTITIONING`
- `252 - DECONV/CONV AMT. LIV. UT.`
- `253 - AIR CONDITIONING - FIREPLACES`
- `254 - BASEMENT-ATTIC ROOM BATH`
- `255 - REPLACE-REPAIR BUIDLING - FACTORY`
- `256 - OTHER REMODELING`
- `257 - REMODELING - INTERIOR ONLY`
- `258 - REMODELING - EXTERIOR ONLY`
- `271 - MAJOR IMPROVEMENT - WRECK`
- `272 - MINOR IMPROVEMENT - WRECK`
- `291 - MAJOR IMPROVEMENT - BURNOUT`
- `292 - MINOR IMPROVEMENT - BURNOUT`
- `300 - RAILROAD PROPERTIES`
- `310 - NEW CONST MAJOR IMPROVEMENT`
- `311 - CARRIER - NEW MAJOR CONSTRUCTION`
- `312 - NON-CARRIER NEW MAJOR CONSTRUCTION`
- `330 - NEW CONSTRUCTION MINOR IMPROVEMENT`
- `331 - CARRIER - REMODELING`
- `332 - NON-CARRIER-REMODELING`
- `350 - REMODELING`
- `351 - CARRIER - REMODELING`
- `352 - NON-CARRIER - REMODELING`
- `370 - WRECKS`
- `371 - MAJOR IMPROVEMENT WRECK`
- `372 - MINOR IMPROVEMENT - WRECK`
- `390 - BURNOUTS`
- `391 - MAJOR IMPROVEMENT - BURNOUT`
- `392 - MINOR IMPROVEMENT - BURNOUT`
- `400 - EXEMPT PROPERTIES`
- `411 - LEASEHOLD`
- `510 - RECHECKS`
- `511 - OCCUPANCY`
- `512 - PRIOR YEAR PARTIAL ASSESSMENT`
- `515 - MANUALLY ISSUED PERMIT`
- `516 - PRIOR YEAR RECHECK`
- `900 - OTHER`
Copy link
Member

Choose a reason for hiding this comment

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

Per our convo the other day, I think we should move the enumerated values of fields to seeds in the ccao DB, then use them for checks against their respective source fields.

- `114.7 - OTHER - MAJOR NEW CONSTRUCTION`
- `114.8 - OTHER - MAJOR NEW CONSTRUCTION`
- `114.9 - OTHER - MAJOR NEW CONSTRUCTION`
- `131 - SWIMING POOL - TENNIS COURT`
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: I assume that the code is this way in the actual enumeration. If so, keep it as-is, if not, let's correct:

Suggested change
- `131 - SWIMING POOL - TENNIS COURT`
- `131 - SWIMMING POOL - TENNIS COURT`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good eye! I double-checked and these values are indeed misspelled in the actual enumeration, so I'm going to leave them as-is 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On second thought, I decided that since we were going to clean up some of these values in 7d94c97, I might as will fix this one too.

@jeancochrane
Copy link
Contributor Author

One open question for you in #597 (comment) @dfsnow, otherwise this should be good to go!

@jeancochrane jeancochrane requested a review from dfsnow September 16, 2024 15:26
Copy link
Member

@dfsnow dfsnow left a comment

Choose a reason for hiding this comment

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

Looks good @jeancochrane. Only minor formatting nitpicks. Feel free to ignore if they don't match the source material.

Comment on lines 135 to 141
- `1: RESIDENTIAL PERMIT`
- `2: COMMERCIAL PERMIT`
- `3: RAILROAD PERMIT`
- `4: EXEMPT PERMIT`
- `5: OFFICE PERMIT`
- `6: OCCUPANCY PERMIT`
- `7: OTHER`
Copy link
Member

Choose a reason for hiding this comment

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

For the sake of consistency:

Suggested change
- `1: RESIDENTIAL PERMIT`
- `2: COMMERCIAL PERMIT`
- `3: RAILROAD PERMIT`
- `4: EXEMPT PERMIT`
- `5: OFFICE PERMIT`
- `6: OCCUPANCY PERMIT`
- `7: OTHER`
- `1 - RESIDENTIAL PERMIT`
- `2 - COMMERCIAL PERMIT`
- `3 - RAILROAD PERMIT`
- `4 - EXEMPT PERMIT`
- `5 - OFFICE PERMIT`
- `6 - OCCUPANCY PERMIT`
- `7 - OTHER`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 7d94c97.

- `234.3 - GARAGE-BARN-BUTLER BUILDING - butler buildings`
- `234.4 - Trash Enclosures`
- `235 - OTHER-MINOR NEW CONSTRUCTION`
- `235.1 - OTHER-MINOR NEW CONSTRUCTION - construction trailers`
Copy link
Member

Choose a reason for hiding this comment

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

nitpick (non-blocking): Again for consistency:

Suggested change
- `235.1 - OTHER-MINOR NEW CONSTRUCTION - construction trailers`
- `235.1 - OTHER - MINOR NEW CONSTRUCTION - construction trailers`

And the same for all below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 7d94c97.

- `519 - CONVERT TO NEW MANUAL`
- `520 - AUDIT DEPT INCREASE TO CHART`
- `530 - RECENT I/C SALES`
- `549 - 1985 HISTORICAL LANDMARK`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `549 - 1985 HISTORICAL LANDMARK`
- `549 - 1985 HISTORICAL LANDMARK`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 7d94c97.

Comment on lines 378 to 383
- `C`: Closed
- `L`: Legacy
- `M`: Manager review
- `O`: Open
- `P`: Pending
- `R`: Recheck
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `C`: Closed
- `L`: Legacy
- `M`: Manager review
- `O`: Open
- `P`: Pending
- `R`: Recheck
- `C - Closed`
- `L - Legacy`
- `M - Manager review`
- `O - Open`
- `P - Pending`
- `R - Recheck`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 7d94c97.

@jeancochrane jeancochrane merged commit a752f44 into master Sep 16, 2024
8 checks passed
@jeancochrane jeancochrane deleted the jeancochrane/239-create-dbt-model-for-permits-view branch September 16, 2024 21:46
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.

Create dbt model for permits view
2 participants