-
Notifications
You must be signed in to change notification settings - Fork 0
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
Comments
@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. |
@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! |
@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. |
@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. |
@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?
The text was updated successfully, but these errors were encountered: