-
Notifications
You must be signed in to change notification settings - Fork 10
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
Interdependent recovery of residential buildings and households #606
Interdependent recovery of residential buildings and households #606
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code runs well besides the minor suggestions. Approved.
pyincore/analyses/residentialbuildingrecovery/residentialbuildingrecovery.py
Outdated
Show resolved
Hide resolved
pyincore/analyses/housingrecoverysequential/housingrecoverysequential.py
Show resolved
Hide resolved
pyincore/analyses/housingrecoverysequential/housingrecoverysequential.py
Show resolved
Hide resolved
…ildings-and-households
start = timer() | ||
|
||
# dev Joplin testbed | ||
population_dislocation = Dataset.from_file( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this a dataset on dev? This is a pretty big file and is better served from the incore-dev and referenced by ID instead.
The test folder should match the analysis folder name. Since the folder is housingrecoverysequential, the test folder shouldn't be housingrecoveryserial it should be housingrecoverysequential. This looks like it was missed before. Can you rename that folder? |
test_housingrecoverysequential_sv.py raises a value error about hhinc, is that expected? |
@@ -352,6 +416,7 @@ def housing_serial_recovery_model( | |||
result["guid"] = households_df["guid"] | |||
result["huid"] = households_df["huid"] | |||
result["Zone"] = households_df["Zone"] | |||
result["Zone Indicator"] = households_df["Zone Indicator"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can probably be shortened to match the 10 char limit for dbf/shapefile. Maybe zone_indic?
Do we need to open issues to add new semantic types for the outputs from residential building recovery? |
@longshuicy Raising a value error if hhinc is not present prevents the analysis from running, should it do that? It looks like the population_dislocation dataset referenced in the test doesn't have that column, probably because the housing unit allocation dataset used to generate it didn't have that column. If hhinc should be there, then we should raise a value error and fix the dataset on dev. Since this isn't a test we execute as part of the pyincore testing, we could fix this later so we can get this merged. |
The original population_dislocation_block (Galveston) doesn't have hhinc and it calculates the zones using social vulnerability. To support Lisa Wang's analysis, Joplin population_dislocation_block has hhinc and zones are calculated using hhinc. So we don't need to raise a value error if hhinc is not there since hhinc is not required. It's only a condition - If hhinc is there, then we add it to pop_dis_selectors Sorry I didn't think it clearly and test properly when I made the changes. |
Just opened an issue #611 |
@navarroc I have addressed all the comments. Thank you! |
Missing "#"
This PR is to support Lisa Wang's new analysis
Interdependent recovery of residential buildings and households
.Two analyses have changes:
sv_generator
from hardcoded to 2 input datasetszone_def_sv
andzone_dev_hhinc
. To make sure analysis runs without inputs, defaultzone_def_sv
andzone_dev_hhinc
are provided.For testing, please run
test_residentialbuildingrecovery.py
test_housingrecoverysequential_sv.py
test_housingrecoverysequential_hhinc.py