Closed tm-jc-nacpil closed 1 year ago
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
View / edit / reply to this conversation on ReviewNB
alronlam commented on 2023-03-24T07:03:30Z ----------------------------------------------------------------
Line #1. aoi_data = gpd.read_file(f'{rollout_date}-{country_code}-rollout-features.geojson')
Would recommend to revert the notebook to be completely runnable from top to bottom without relying on loading data from intermediate files.
View / edit / reply to this conversation on ReviewNB
alronlam commented on 2023-03-24T07:03:31Z ----------------------------------------------------------------
Line #3. q_decimal = q / 100
Since we decided not to do this anymore, would recommend just deleting these quantile configs to reeduce complexity.
View / edit / reply to this conversation on ReviewNB
alronlam commented on 2023-03-24T07:03:32Z ----------------------------------------------------------------
Line #4. max_val = features[[col]].quantile(q_decimal)
Same comment re: quantile. Let's just remove it.
View / edit / reply to this conversation on ReviewNB
alronlam commented on 2023-03-24T07:03:33Z ----------------------------------------------------------------
Line #1. # Uncomment this cell and run to save a local copy of the scaled features
Since you said uncomment, might want to make the code commented by default haha
View / edit / reply to this conversation on ReviewNB
alronlam commented on 2023-03-24T07:03:34Z ----------------------------------------------------------------
Please remove this whole part; our asset index does not directly relate to income and socio-economic class.
tm-jc-nacpil commented on 2023-03-24T07:25:04Z ----------------------------------------------------------------
Hi alron, do you mean removing even the binning step entirely or just the markdown explanation?
alronlam commented on 2023-03-24T08:35:59Z ----------------------------------------------------------------
I meant just the markdown explanation (and the split-quintile parts). But retain the quintiles.
View / edit / reply to this conversation on ReviewNB
alronlam commented on 2023-03-24T07:03:35Z ----------------------------------------------------------------
Delete this whole split-quintile category section too.
View / edit / reply to this conversation on ReviewNB
alronlam commented on 2023-03-24T07:03:36Z ----------------------------------------------------------------
Line #2. rollout_aoi.to_file(f'{rollout_date}-{country_code}-rollout-output-minmax-q{q}.geojson', driver='GeoJSON', index=False)
1. What is q
?
2. Double checking, this file also contain population counts right?
3. Noting for code clean-up sprint, we might want to refactor so that we save a copy of:
This will make any further EDAs much faster, and allows anybody to do so without having to re-run Indonesia.
Illustrative example based on the single country notebooks:
# Join back the features rollout_output_with_unscaled_features = rollout_aoi.join(features) <save this file> <do the same for scaled>
tm-jc-nacpil commented on 2023-03-24T08:32:17Z ----------------------------------------------------------------
Done!
Hi alron, do you mean removing even the binning step entirely or just the markdown explanation?
View entire conversation on ReviewNB
I meant just the markdown explanation (and the split-quintile parts). But retain the quintiles.
View entire conversation on ReviewNB
This notebook updates the Indonesia rollout output and comparison
Changes
Results