-
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
Allow custom shapes #20
base: main
Are you sure you want to change the base?
Changes from 5 commits
def1072
b8f2dec
251a02a
a422b25
78c56ae
f68b720
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,3 +6,6 @@ dependencies: | |
- python=3.6 | ||
- snakemake-minimal=5.4 | ||
- pycountry=20.7.3 | ||
- pip | ||
- pip: | ||
- -e ./lib | ||
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. The lib requires 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'm not in favour of installing the lib into the base env, as this env should be as lean as possible. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,4 +19,4 @@ dependencies: | |
- pycountry=20.7.3 | ||
- pip=19.1.1 | ||
- pip: | ||
- -e ../../lib | ||
- -e ../../lib[geo] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,3 @@ | ||
"""Top-level package of renewablepotentialslib.""" | ||
|
||
__version__ = "0.1.0" # Additionally defined in setup.py. | ||
|
||
# from https://epsg.io/3035 | ||
EPSG_3035 = "EPSG:3035" | ||
EPSG_3035_PROJ4 = "+proj=laea +lat_0=52 +lon_0=10 +x_0=4321000 +y_0=3210000 +ellps=GRS80 +units=m +no_defs " | ||
# from https://epsg.io/4326 | ||
WGS84 = "EPSG:4326" | ||
WGS84_PROJ4 = "+proj=longlat +datum=WGS84 +no_defs " |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
# from https://epsg.io/3035 | ||
EPSG_3035 = "EPSG:3035" | ||
EPSG_3035_PROJ4 = "+proj=laea +lat_0=52 +lon_0=10 +x_0=4321000 +y_0=3210000 +ellps=GRS80 +units=m +no_defs " | ||
# from https://epsg.io/4326 | ||
WGS84 = "EPSG:4326" | ||
WGS84_PROJ4 = "+proj=longlat +datum=WGS84 +no_defs " |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
def collect_shape_dirs(config, rules): | ||
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. Being in the public API of the 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. You may not like this comment: I don't think this function should be in the lib. This is for three reasons:
For these reasons, I believe the function should live in 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, it's where I first put it, for all the reasons you give above. However, I can't test it when it sits in the middle of the Snakefile, I assume... Perhaps a 'rule_utils.py' file in the |
||
STRIP = "0123456789" | ||
shapes = set(shape.rstrip(STRIP) for layer in config["layers"].values() for shape in layer.values()) | ||
inputs = {} | ||
for shape in shapes: | ||
if shape in ["nuts", "gadm", "lau"]: | ||
inputs[shape] = getattr(rules, f"administrative_borders_{shape}").output[0] | ||
else: | ||
inputs[shape] = config["data-sources"][shape] | ||
return inputs |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,14 +10,17 @@ | |
maintainer_email='[email protected]', | ||
packages=find_packages(exclude=['tests*']), | ||
include_package_data=True, | ||
install_requires=[ | ||
"pycountry", | ||
"geopandas", | ||
"numpy", | ||
"pandas", | ||
"shapely", | ||
"pyproj" | ||
], | ||
install_requires=["numpy"], | ||
extras_require={ | ||
'geo': [ | ||
"pycountry", | ||
"geopandas", | ||
"numpy", | ||
"pandas", | ||
"shapely", | ||
"pyproj" | ||
], | ||
}, | ||
classifiers=[ | ||
'Environment :: Console', | ||
'Intended Audience :: Science/Research', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,4 +13,4 @@ dependencies: | |
- pandas=0.23.2 | ||
- shapely=1.7.1 | ||
- pip: | ||
-e . | ||
- -e .[geo] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ | |
import pandas as pd | ||
from pandas.testing import assert_frame_equal | ||
|
||
from renewablepotentialslib.conversion import ( | ||
from renewablepotentialslib.geo.conversion import ( | ||
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 recommend to rename the parent folder of this test to |
||
watt_to_watthours, | ||
eu_country_code_to_iso3, | ||
coordinate_string_to_decimal, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
import pytest | ||
|
||
from renewablepotentialslib.rule_utils import collect_shape_dirs | ||
|
||
|
||
class TestCustomShapes: | ||
@pytest.fixture | ||
def rules(self): | ||
class DotDict(dict): | ||
"""dot.notation access to dictionary attributes""" | ||
def __getattr__(*args): | ||
val = dict.__getitem__(*args) | ||
return DotDict(val) if type(val) is dict else val | ||
__setattr__ = dict.__setitem__ | ||
__delattr__ = dict.__delitem__ | ||
return DotDict({ | ||
"administrative_borders_nuts": {"output": ["you_got_nuts"]}, | ||
"administrative_borders_gadm": {"output": ["you_got_gadm"]}, | ||
"administrative_borders_lau": {"output": ["you_got_lau"]} | ||
}) | ||
|
||
@pytest.mark.parametrize("number", (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 100, 1000, 5555)) | ||
def test_strip(self, rules, number): | ||
config = {"layers": {"national": {"foo": "nuts{}".format(number)}}} | ||
collected = collect_shape_dirs(config, rules) | ||
assert collected == {"nuts": "you_got_nuts"} | ||
|
||
@pytest.mark.parametrize("number", (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 100, 1000, 5555)) | ||
def test_no_strip(self, rules, number): | ||
config = {"layers": {"national": {"foo": "{0}bar{0}".format(number)}}, "data-sources": {f"{number}bar": "foobar"}} | ||
collected = collect_shape_dirs(config, rules) | ||
assert collected == {f"{number}bar": "foobar"} | ||
|
||
def test_standard_shapes(self, rules): | ||
config = { | ||
"layers": { | ||
"national": {"foo": "nuts2", "bar": "nuts3", "baz": "gadm0"}, | ||
"custom": {"foo": "nuts2", "bar": "lau2", "baz": "gadm0"} | ||
}, | ||
"data-sources": {"my-custom-shape": "data/dir.geojson"} | ||
} | ||
collected = collect_shape_dirs(config, rules) | ||
assert set(collected.keys()) == set(["lau", "nuts", "gadm"]) | ||
for key in ["lau", "nuts", "gadm"]: | ||
assert collected[key] == f"you_got_{key}" | ||
|
||
def test_new_shapes(self, rules): | ||
config = { | ||
"layers": { | ||
"national": {"foo": "my-custom-shape1", "bar": "nuts3", "baz": "my-custom-shape2"}, | ||
"custom": {"foo": "your-custom-shape2", "bar": "my-custom-shape12", "baz": "gadm0"} | ||
}, | ||
"data-sources": {"my-custom-shape": "mydata/dir.geojson", "your-custom-shape": "yourdata/dir.geojson"} | ||
} | ||
collected = collect_shape_dirs(config, rules) | ||
assert set(collected.keys()) == set(["my-custom-shape", "your-custom-shape", "nuts", "gadm"]) | ||
for key in ["my", "your"]: | ||
assert collected[f"{key}-custom-shape"] == f"{key}data/dir.geojson" | ||
for key in ["nuts", "gadm"]: | ||
assert collected[key] == f"you_got_{key}" | ||
|
||
def test_will_fail(self, rules): | ||
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. Can you give this test a more self-explanatory name? I don't understand why this is failing. Is it because of the dashes? |
||
config = { | ||
"layers": { | ||
"national": {"foo": "my-custom-shape", "bar": "nuts3"}, | ||
}, | ||
"data-sources": {"your-custom-shape": "yourdata/dir.geojson"} | ||
} | ||
with pytest.raises(KeyError) as excinfo: | ||
collected = set(collect_shape_dirs(config, rules).keys()) | ||
|
||
assert "my-custom-shape" in str(excinfo.value) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import geopandas as gpd | ||
|
||
from renewablepotentialslib.shape_utils import ( | ||
from renewablepotentialslib.geo.shape_utils import ( | ||
buffer_if_necessary, | ||
to_multi_polygon, | ||
drop_countries, | ||
|
@@ -22,12 +22,10 @@ | |
} | ||
|
||
|
||
def normalise_admin_borders(path_to_nuts, path_to_gadm, path_to_lau, crs, scope_config, path_to_output): | ||
def normalise_admin_borders(crs, scope_config, path_to_output, **shape_dirs): | ||
"""Normalises raw administrative boundary data and places it in one, layered geodatapackage.""" | ||
|
||
for _src, _path in { | ||
'nuts': path_to_nuts, 'gadm': path_to_gadm, 'lau': path_to_lau | ||
}.items(): | ||
for _src, _path in shape_dirs.items(): | ||
gdf = gpd.read_file(_path) | ||
gdf = gdf.to_crs(crs) | ||
gdf.geometry = gdf.geometry.map(buffer_if_necessary).map(to_multi_polygon) | ||
|
@@ -47,11 +45,10 @@ def normalise_admin_borders(path_to_nuts, path_to_gadm, path_to_lau, crs, scope_ | |
|
||
|
||
if __name__ == "__main__": | ||
shape_dirs = {k: v for k, v in snakemake.input.items() if k != "src"} | ||
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. This seems brittle. Isn't it possible to give the whole thing a name? So instead of 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'm not sure that snakemake can track inputs that are defined inside a nested dictionary... |
||
normalise_admin_borders( | ||
path_to_nuts=snakemake.input.nuts_geojson, | ||
path_to_gadm=snakemake.input.gadm_gpkg, | ||
path_to_lau=snakemake.input.lau_gpkg, | ||
crs=snakemake.params.crs, | ||
scope_config=snakemake.params.scope, | ||
path_to_output=snakemake.output[0] | ||
path_to_output=snakemake.output[0], | ||
**shape_dirs | ||
) |
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 is a pity. Not really bad though.