-
Notifications
You must be signed in to change notification settings - Fork 2
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
Update study area to account for exclusion zones #5
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
import fiona.transform | ||
import geopandas as gpd | ||
import shapely.geometry | ||
import shapely.errors | ||
from shapely.prepared import prep | ||
|
||
from src.utils import Config | ||
|
@@ -31,12 +32,7 @@ | |
@click.argument("path_to_output") | ||
@click.argument("config", type=Config()) | ||
def retrieve_administrative_borders(path_to_countries, max_layer_depths, path_to_output, config): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not convinced by this configuration mechanism (which I introduced and) which we are extending here. But I guess it may not the right moment to fix that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I didn't edit the config mechanism. Best to fix it universally in a different PR |
||
study_area = shapely.geometry.box( | ||
minx=config["scope"]["bounds"]["x_min"], | ||
maxx=config["scope"]["bounds"]["x_max"], | ||
miny=config["scope"]["bounds"]["y_min"], | ||
maxy=config["scope"]["bounds"]["y_max"] | ||
) | ||
study_area = _study_area(config) | ||
study_area = prep(study_area) # improves performance | ||
with fiona.open(path_to_countries[0], "r", layer=0) as first_country: | ||
src_crs = first_country.crs | ||
|
@@ -78,6 +74,41 @@ def _country_features(path_to_file, layer_id, study_area): | |
yield new_feature | ||
|
||
|
||
def _study_area(config): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this maybe move to In that case I'd not hand the config over to it, but rather a |
||
""" | ||
Create a bounding box for the study area, and cut out holes for all defined | ||
exclusion zones. For plotting purposes, exclusion zones and the bounding box are | ||
defined in opposite orientations, see https://github.com/geopandas/geopandas/issues/951 | ||
""" | ||
if config["scope"].get("exclusion_zones", {}) and isinstance(config["scope"]["exclusion_zones"], dict): | ||
holes = [ | ||
( | ||
(exclusion_zone["x_max"], exclusion_zone["y_min"]), | ||
(exclusion_zone["x_max"], exclusion_zone["y_max"]), | ||
(exclusion_zone["x_min"], exclusion_zone["y_max"]), | ||
(exclusion_zone["x_min"], exclusion_zone["y_min"]) | ||
) | ||
for exclusion_zone in config["scope"]["exclusion_zones"].values() | ||
] | ||
else: | ||
holes = [] | ||
|
||
study_area = shapely.geometry.Polygon( | ||
((config["scope"]["bounds"]["x_min"], config["scope"]["bounds"]["y_min"]), | ||
(config["scope"]["bounds"]["x_min"], config["scope"]["bounds"]["y_max"]), | ||
(config["scope"]["bounds"]["x_max"], config["scope"]["bounds"]["y_max"]), | ||
(config["scope"]["bounds"]["x_max"], config["scope"]["bounds"]["y_min"])), | ||
holes=holes | ||
) | ||
if study_area.is_valid is False: | ||
raise shapely.errors.TopologicalError( | ||
"Invalid study area geometry. " | ||
"Ensure that exclusion zones do not share a border with the study bounds." | ||
) | ||
else: | ||
return study_area | ||
|
||
|
||
def _in_study_area(study_area): | ||
def _in_study_area(feature): | ||
unit = shapely.geometry.shape(feature["geometry"]) | ||
|
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.
If you change the
default.yaml
, I think you should test whether the entire workflow still works. Alternatively, create another config file. Even then, it would be good to test the workflow.The package conflicts should be fixed at the moment.
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.
sure thing. I haven't checked the functioning of the workflow in this repo on any of these PRs, I have to admit...