NYCPlanning / db-pluto

🔭 PLUTO and MapPLUTO
https://nycplanning.github.io/db-pluto
38 stars 13 forks source link

Switch to just using PLUTO manual research for corrections #425

Closed AmandaDoyle closed 1 year ago

AmandaDoyle commented 1 year ago

Addresses issue #424

mbh329 commented 1 year ago

Otherwise this makes sense to me

damonmcc commented 1 year ago

@AmandaDoyle I'm seeing more uses of the to-be-deprecated pluto_input_research table (example in corr_communitydistricts.sql). but this is a WIP so no worries if those are todos!

and maybe these changes be tested later by building on this branch and comparing to 22v3.1?

AmandaDoyle commented 1 year ago

Latest changes do the following

damonmcc commented 1 year ago

just kicked off a build to test the changes

if it works and the output is identical to 22v3.1 from main branch, would you wanna delete the commented out code? should be easy enough to revert changes via git/github if we have to

damonmcc commented 1 year ago

@AmandaDoyle

there's a QAQC app error since an expected pluto_corrections export doesn't exist. I opened an issue for it and can work on it asap

we probably already mentioned this but this error is reminding me that the new research/corrections/change process maybe won't be implemented until 23v1. does that sounds right?

AmandaDoyle commented 1 year ago

@damonmcc thanks for flagging. Working through issue #251 would be good, but if there implications to previous versions of PLUTO (i.e. QAQC reports of 22v2 wouldn't work), perhaps we don't rename the file, or scope out what needs to be changed?

That's right, that the earliest this change would be implemented is for 23v1, so this isn't pressing.

mbh329 commented 1 year ago

It seems like the the issue to get the right corrections would need to be fixed in this function function. could we just implement an if/else statement in this function? might be good to talk this through

damonmcc commented 1 year ago

I'll compare 22v3.1 pluto_changes_applied.csv generated by this branch to 22v3.1 pluto_corrections_applied.csv generated on main

can use 22v3 pluto_corrections_applied.csv as a baseline file

damonmcc commented 1 year ago

@AmandaDoyle @mbh329

When comparing 22v3 to 22v3.1 applied changes, I'm seeing only one type of difference in applied changes:

image

When comparing 22v3.1 main branch to 22v3.1 this branch applied changes, I'm seeing many types of differences in applied changes:

a concern: I'm seeing an older version change for the same BBL/field beat a newer one when they're both on pluto_input_research. it's the changes for 1019570140,cd. they're actually the same change but differ in reason and version

damonmcc commented 1 year ago

check to do: compare non-programatic field changes from applied.csv + not_applied.csv and ensure they match those in pluto_unput_research.csv

so far seeing

AmandaDoyle commented 1 year ago

@damonmcc and @mbh329

I don't see anything that need to be changed on the code side before merging this in but before merging this in I'd like to check with LS because this will have implications on how we communicate how we make changes and the scope of changes, and that she's prepared for that for the upcoming 23v1 build.

AmandaDoyle commented 1 year ago

@damonmcc and @mbh329 LS is okay merging this in and having these changes be applied to 23v1. I need you to verify that the changes on code and outputs are good to go and pass your review. Reminder that this is a maintenance item this sprint.