search

NYCPlanning / db-developments

šŸ  šŸ˜ļø šŸ—ļø Developments Database
https://nycplanning.github.io/db-developments
8 stars 2 forks source link

QAQC Manual Corrections Addition #536

Closed td928 closed 2 years ago

td928 commented 2 years ago

532 one reviewer is good šŸ‰

I feel like I lost some steam half way through this because I realize the tables the pipelines generated for manual corrections qaqc are really good! (corrections_applied and corrections_not_applied). Also keeping in mind that anything we want to add to the app is really to benefit our team members, so I really can only come up with justifications for the two checks added below.

maunal_hny_match_check

this is to see if our pipeline added manual corrections from CORR_hny_matches adding to a job number was successful. It is pretty simple but should raise a flag because that can indicate the step to incorporate manual corrections for hny, which is distinct from the other manual corrections ingestions, might have failed somehow.

manual_corrections_not_applied

pull in this table maybe just to have a total count in our app to make sure this number is somewhat stable. Again the actual job numbers are probably still only useful to the subject matter experts.

One last overall comment/question is I am hoping the place I added in the query makes some sense in the overall qaqc file creation process.

Oysters1874 commented 2 years ago

we should merge this into dev, right?

mbh329 commented 2 years ago

@td928 @Oysters1874 - yes, should be merging into dev not main

Oysters1874 commented 2 years ago

Also, shouldn't we keep all these three checks classa_net_mismatch, manual_hny_match_check, and manual_corrections_not_applied as separate things? Instead of selecting job_number from qaqc_app_additions each time we adding a new column, I feel like we should select job_number from final_devdb. # of records with manual_corrections_not_applied equals to 1 seems do not match with # of records in corrections_not_applied.

td928 commented 2 years ago

@mbh329 I guess this is kind of my last point in my initial comment. They would be all eventually merged into the same qaqc_app file so really it is just for clarity. I do wonder if they are supposed in different files then whether qaqc_app_addtional.sql should be more specific ie. classa_net_mismatch as well and maybe should not exist in the first place.