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

resolve sqft_per_unit and building_sqft_per_unit #169

Open
stefancoe opened this issue Jan 15, 2019 · 4 comments
Open

resolve sqft_per_unit and building_sqft_per_unit #169

stefancoe opened this issue Jan 15, 2019 · 4 comments

Comments

@stefancoe
Copy link
Contributor

@hanase sqft_per_unit is a local column on the building table and building_sqft_per_unit is a computed variable on the building table. I think we should stick with one as some dependencies are using one or the other. Perhaps not include sqft_per_unit when importing data and just compute it?

@hanase
Copy link
Collaborator

hanase commented Jan 16, 2019

@stefancoe , the sqft_per_unit is used to determine residential sqft, which is residential_units * sqft_per_unit. Without that column in the table, you could not determine building_sqft.

I think the confusion is caused by poor naming of the variables. building_sqft_per_unit is actually total_building_sqft_per_residential_unit which is different than the sqft_per_unit column.

We should revisit all the uses and maybe rename the variables.

@stefancoe
Copy link
Contributor Author

@hanase Thanks for the clarification! When/why would we want to use building_sqft_per_unit (which potentially includes non-residential square footage, right?) as opposed to sqft_per_unit. I see that it is used in price_per_unit, which is then used by ln_price_residual. Thanks!

@hanase
Copy link
Collaborator

hanase commented Jan 17, 2019

@stefancoe This might have been the reason why we switched everything to "per sqft". Let's find the various uses and discuss it in our meeting on Monday.

@stefancoe
Copy link
Contributor Author

@hanase Sounds good. I'll look at all the places it's used- seems like we should be using sqft_per_unit_imputed instead, at least when dealing with variables that are representing residential components of a building.

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

No branches or pull requests

2 participants