search

NYCPlanning / db-equitable-development-tool

Data Repo for the equitable development tool (EDDT)
MIT License
0 stars 0 forks source link

Renaming and Adjusting Housing Production Columns #231

Closed td928 closed 2 years ago

td928 commented 2 years ago

Overview

very simple work to mostly add _count token to housing production indicators (only three: change in units, historic districts and hpd affordable housing). Also a quickly reordering columns for the historic district (only three columns) so the denom comes last.

SashaWeinstein commented 2 years ago

The big issue here is that the branch you worked off is old and way behind dev. The general indicator tests aren't on this branch. Can you merge in dev? Then I will review

SashaWeinstein commented 2 years ago

The branch with my work on this improvement is 223-Count-Token-Housing-Prod, which is more up to date

td928 commented 2 years ago

The big issue here is that the branch you worked off is old and way behind dev. The general indicator tests aren't on this branch. Can you merge in dev? Then I will review

Hey Sasha not sure what you said was true. I merged in dev before starting on this. And from only 10 files change compared to dev I think it seems merge was successful.

SashaWeinstein commented 2 years ago

sorry I'm a dummy I forgot to pull after checking out the feature branch

SashaWeinstein commented 2 years ago

Ok I was wrong about the branch being out of date but I had already done some of this work on 223-Count-Token-Housing-Prod. I thought this was clear from context as all the issues this improvement were made around the time. We can take your solution but we need to be more careful about this in the future

td928 commented 2 years ago

Ok I was wrong about the branch being out of date but I had already done some of this work on 223-Count-Token-Housing-Prod. I thought this was clear from context as all the issues this improvement were made around the time. We can take your solution but we need to be more careful about this in the future

yeah I should pay more attention to the issue numbers. But I was thrown off by the default git fetch did not return 223-Count-Token-Housing-Prod branch so I went to the most plausible one next in line. Probably should ask you first.

SashaWeinstein commented 2 years ago

Ok I was wrong about the branch being out of date but I had already done some of this work on 223-Count-Token-Housing-Prod. I thought this was clear from context as all the issues this improvement were made around the time. We can take your solution but we need to be more careful about this in the future

yeah I should pay more attention to the issue numbers. But I was thrown off by the default git fetch did not return 223-Count-Token-Housing-Prod branch so I went to the most plausible one next in line. Probably should ask you first.

ok yes I see I never pushed that branch. so definitely my fault, I assumed that it was pushed and that you knew I had started on this improvement but neither of those things were true

td928 commented 2 years ago

yeah I can l do a better job communicating next time! Took your suggestion for list comprehension change and test it the outlook look good.