-
Notifications
You must be signed in to change notification settings - Fork 1
run script for webmap geojsons #138
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
base: dev
Are you sure you want to change the base?
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.
Thanks so much for this @danlewis85 !
It looks good and I couldn't find any errors. I left some comments mostly re documentation, plus some minor suggestions.
I would suggest moving your functions in this script into some utils
files. We can make a separate issue for this if easier. I think they could be useful for other stuff as well.
Also, some of the functions could be renamed to be clearer. Our repo style guide suggests the following format for readability: action_returnsType_description
e.g. check_exists_str_file_uri
instead of get_file_uri
.
Question - is there a separate script for the geojson creation and simplification which needs review?
Thanks so much :)
asf_heat_pump_suitability/pipeline/run_scripts/run_make_webmap_geojsons.py
Outdated
Show resolved
Hide resolved
asf_heat_pump_suitability/pipeline/run_scripts/run_make_webmap_geojsons.py
Show resolved
Hide resolved
asf_heat_pump_suitability/pipeline/run_scripts/run_make_webmap_geojsons.py
Outdated
Show resolved
Hide resolved
asf_heat_pump_suitability/pipeline/run_scripts/run_make_webmap_geojsons.py
Outdated
Show resolved
Hide resolved
asf_heat_pump_suitability/pipeline/run_scripts/run_make_webmap_geojsons.py
Outdated
Show resolved
Hide resolved
|
||
# Load scores file | ||
logger.info(f"Reading suitability scores from {file_uri}") | ||
scores = pd.read_parquet(file_uri) |
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.
pandas jumpscare lol
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.
I don't understand this - leaving open for for info.
asf_heat_pump_suitability/pipeline/run_scripts/run_make_webmap_geojsons.py
Outdated
Show resolved
Hide resolved
asf_heat_pump_suitability/pipeline/run_scripts/run_make_webmap_geojsons.py
Show resolved
Hide resolved
asf_heat_pump_suitability/pipeline/run_scripts/run_make_webmap_geojsons.py
Outdated
Show resolved
Hide resolved
asf_heat_pump_suitability/pipeline/run_scripts/run_make_webmap_geojsons.py
Outdated
Show resolved
Hide resolved
…_geojsons.py Co-authored-by: crispy-wonton <104171770+crispy-wonton@users.noreply.github.com>
…_geojsons.py Co-authored-by: crispy-wonton <104171770+crispy-wonton@users.noreply.github.com>
…pump_suitability into 74_tippecanoe_geojsons Merging upstream minor stylistic changes.
Description
This PR add a new run script /pipeline/run_scripts/run_make_webmap_geojsons.py and some associated keys in the config file (under the
webmap_data_source
key).Running this script produces a set of GB-level geojson files with suitability scores appended. The geojsons produced are at different levels of aggregation to allow for effective rendering at different levels of zoom.
Closes #74
Instructions for Reviewer
Could you check if the doc string at the top of the run script makes sense.
Can you try running the run_make_webmap_geojsons.py script.
Can you ensure you are happy with the logic of the script.
Checklist:
notebooks/
pre-commit
and addressed any issues not automatically fixeddev
README
s