Skip to content

Commit

Permalink
Merge branch 'low-hanging-refactors' into harmony
Browse files Browse the repository at this point in the history
  • Loading branch information
trey-stafford committed Nov 11, 2024
2 parents f5b4214 + 77112b7 commit 0b2f704
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 82 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/integration_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,6 @@ jobs:
pytest icepyx/tests/integration --verbose --cov app
- name: "Upload coverage report"
uses: "codecov/codecov-action@v4.5.0"
uses: "codecov/codecov-action@v4.6.0"
with:
token: "${{ secrets.CODECOV_TOKEN }}"
2 changes: 1 addition & 1 deletion .github/workflows/unit_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ jobs:
--ignore=icepyx/tests/integration
- name: "Upload coverage report"
uses: "codecov/codecov-action@v4.5.0"
uses: "codecov/codecov-action@v4.6.0"
with:
token: "${{ secrets.CODECOV_TOKEN }}"
180 changes: 110 additions & 70 deletions icepyx/core/spatial.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,86 @@
ExtentType = Literal["bounding_box", "polygon"]


def _convert_spatial_extent_to_list_of_floats(
spatial_extent: Union[list[float], list[tuple[float, float]], Polygon],
) -> list[float]:
# This is already a list of floats
if isinstance(spatial_extent, list) and isinstance(spatial_extent[0], float):
spatial_extent = cast(list[float], spatial_extent)
return spatial_extent
elif isinstance(spatial_extent, Polygon):
# Convert `spatial_extent` into a list of floats like:
# `[longitude1, latitude1, longitude2, latitude2, ...]`
spatial_extent = [
float(coord) for point in spatial_extent.exterior.coords for coord in point
]
return spatial_extent
elif isinstance(spatial_extent, list) and isinstance(spatial_extent[0], tuple):
# Convert the list of tuples into a flat list of floats
spatial_extent = cast(list[tuple[float, float]], spatial_extent)
spatial_extent = list(chain.from_iterable(spatial_extent))
return spatial_extent
else:
raise TypeError(
"Unrecognized spatial_extent that"
" cannot be converted into a list of floats:"
f"{spatial_extent=}"
)


def _geodataframe_from_bounding_box(
spatial_extent: list[float],
xdateline: bool,
) -> gpd.GeoDataFrame:
if xdateline is True:
cartesian_lons = [i if i > 0 else i + 360 for i in spatial_extent[0:-1:2]]
cartesian_spatial_extent = [
item for pair in zip(cartesian_lons, spatial_extent[1::2]) for item in pair
]
bbox = box(
cartesian_spatial_extent[0],
cartesian_spatial_extent[1],
cartesian_spatial_extent[2],
cartesian_spatial_extent[3],
)
else:
bbox = box(
spatial_extent[0],
spatial_extent[1],
spatial_extent[2],
spatial_extent[3],
)

# TODO: test case that ensures gdf is constructed as expected (correct coords, order, etc.)
# HACK: Disabled Pyright due to issue
# https://github.com/geopandas/geopandas/issues/3115
return gpd.GeoDataFrame(geometry=[bbox], crs="epsg:4326") # pyright: ignore[reportCallIssue]


def _geodataframe_from_polygon_list(
spatial_extent: list[float],
xdateline: bool,
) -> gpd.GeoDataFrame:
if xdateline is True:
cartesian_lons = [i if i > 0 else i + 360 for i in spatial_extent[0:-1:2]]
spatial_extent = [
item for pair in zip(cartesian_lons, spatial_extent[1::2]) for item in pair
]

spatial_extent_geom = Polygon(
# syntax of dbl colon is- "start:stop:steps"
# 0::2 = start at 0, grab every other coord after
# 1::2 = start at 1, grab every other coord after
zip(spatial_extent[0::2], spatial_extent[1::2])
) # spatial_extent
# TODO: check if the crs param should always just be epsg:4326 for everything OR if it should be a parameter
# HACK: Disabled Pyright due to issue
# https://github.com/geopandas/geopandas/issues/3115
return gpd.GeoDataFrame( # pyright: ignore[reportCallIssue]
index=[0], crs="epsg:4326", geometry=[spatial_extent_geom]
)


def geodataframe(
extent_type: ExtentType,
spatial_extent: Union[str, list[float], list[tuple[float, float]], Polygon],
Expand Down Expand Up @@ -69,6 +149,7 @@ def geodataframe(
0 POLYGON ((-48 68, -48 71, -55 71, -55 68, -48 ...
Name: geometry, dtype: geometry
"""
# DevGoal: the crs setting and management needs to be improved

# If extent_type is a polygon AND from a file, create a geopandas geodataframe from it
if file is True:
Expand All @@ -82,84 +163,38 @@ def geodataframe(
f"Expected list of floats, list of tuples of floats, or Polygon, received {spatial_extent=}"
)

if isinstance(spatial_extent, Polygon):
# Convert `spatial_extent` into a list of floats like:
# `[longitude1, latitude1, longitude2, latitude2, ...]`
spatial_extent = [
float(coord) for point in spatial_extent.exterior.coords for coord in point
]

# We are dealing with a `list[tuple[float, float]]`
if isinstance(spatial_extent, list) and isinstance(spatial_extent[0], tuple):
# Convert the list of tuples into a flat list of floats
spatial_extent = cast(list[tuple[float, float]], spatial_extent)
spatial_extent = list(chain.from_iterable(spatial_extent))

spatial_extent = cast(list[float], spatial_extent)

if xdateline is not None:
xdateline = xdateline
else:
xdateline = check_dateline(extent_type, spatial_extent)

if extent_type == "bounding_box":
if xdateline is True:
cartesian_lons = [i if i > 0 else i + 360 for i in spatial_extent[0:-1:2]]
cartesian_spatial_extent = [
item
for pair in zip(cartesian_lons, spatial_extent[1::2])
for item in pair
]
bbox = box(
cartesian_spatial_extent[0],
cartesian_spatial_extent[1],
cartesian_spatial_extent[2],
cartesian_spatial_extent[3],
)
else:
bbox = box(
spatial_extent[0],
spatial_extent[1],
spatial_extent[2],
spatial_extent[3],
)

# TODO: test case that ensures gdf is constructed as expected (correct coords, order, etc.)
# HACK: Disabled Pyright due to issue
# https://github.com/geopandas/geopandas/issues/3115
return gpd.GeoDataFrame(geometry=[bbox], crs="epsg:4326") # pyright: ignore[reportCallIssue]
#### Non-file processing
# Most functions that this function calls requires the spatial extent as a
# list of floats. This function provides that.
spatial_extent_list = _convert_spatial_extent_to_list_of_floats(
spatial_extent=spatial_extent,
)

if xdateline is None:
xdateline = check_dateline(
extent_type,
spatial_extent_list,
)

# DevGoal: Currently this if/else within this elif are not tested...
# DevGoal: the crs setting and management needs to be improved
if extent_type == "bounding_box":
return _geodataframe_from_bounding_box(
spatial_extent=spatial_extent_list,
xdateline=xdateline,
)

elif extent_type == "polygon":
# if spatial_extent is already a Polygon
if isinstance(spatial_extent, Polygon):
spatial_extent_geom = spatial_extent
return gpd.GeoDataFrame( # pyright: ignore[reportCallIssue]
index=[0], crs="epsg:4326", geometry=[spatial_extent_geom]
)

# else, spatial_extent must be a list of floats (or list of tuples of floats)
else:
if xdateline is True:
cartesian_lons = [
i if i > 0 else i + 360 for i in spatial_extent[0:-1:2]
]
spatial_extent = [
item
for pair in zip(cartesian_lons, spatial_extent[1::2])
for item in pair
]

spatial_extent_geom = Polygon(
# syntax of dbl colon is- "start:stop:steps"
# 0::2 = start at 0, grab every other coord after
# 1::2 = start at 1, grab every other coord after
zip(spatial_extent[0::2], spatial_extent[1::2])
) # spatial_extent
# TODO: check if the crs param should always just be epsg:4326 for everything OR if it should be a parameter
# HACK: Disabled Pyright due to issue
# https://github.com/geopandas/geopandas/issues/3115
return gpd.GeoDataFrame( # pyright: ignore[reportCallIssue]
index=[0], crs="epsg:4326", geometry=[spatial_extent_geom]
# The input must be a list of floats.
return _geodataframe_from_polygon_list(
spatial_extent=spatial_extent_list,
xdateline=xdateline,
)

else:
Expand All @@ -171,6 +206,8 @@ def geodataframe(

def check_dateline(
extent_type: ExtentType,
# TODO: I think this is actually wrong. It expects a different type of
# spatial_extent depending on the `extent_type`, showing below.
spatial_extent: list[float],
) -> bool:
"""
Expand All @@ -193,6 +230,7 @@ def check_dateline(
indicating whether or not the spatial extent crosses the dateline.
"""
if extent_type == "bounding_box":
# We expect the bounding_box to be a list of floats.
if spatial_extent[0] > spatial_extent[2]:
# if lower left lon is larger then upper right lon, verify the values are crossing the dateline
assert spatial_extent[0] - 360 <= spatial_extent[2]
Expand All @@ -208,6 +246,8 @@ def check_dateline(

# this works properly, but limits the user to at most 270 deg longitude...
elif extent_type == "polygon":
# This checks that the first instance of `spatial_extent` NOT a list or
# a tuple. Assumes that this is a list of floats.
assert not isinstance(
spatial_extent[0], (list, tuple)
), "Your polygon list is the wrong format for this function."
Expand Down
32 changes: 23 additions & 9 deletions icepyx/tests/unit/test_spatial.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import pytest
from shapely.geometry import Polygon

import icepyx
import icepyx as ipx
import icepyx.core.spatial as spat

# ######### "Bounding Box" input tests ################################################################################
Expand Down Expand Up @@ -386,8 +386,14 @@ def test_bad_poly_inputfile_type_throws_error():


def test_gdf_from_one_bbox():
obs = spat.geodataframe("bounding_box", [-55, 68, -48, 71])
geom = [Polygon(list(zip([-55, -55, -48, -48, -55], [68, 71, 71, 68, 68])))]
obs = spat.geodataframe("bounding_box", [-55.0, 68.0, -48.0, 71.0])
geom = [
Polygon(
list(
zip([-55.0, -55.0, -48.0, -48.0, -55.0], [68.0, 71.0, 71.0, 68.0, 68.0])
)
)
]
exp = gpd.GeoDataFrame(geometry=geom)

# make sure there is only one geometry before comparing them
Expand All @@ -397,8 +403,14 @@ def test_gdf_from_one_bbox():


def test_gdf_from_multi_bbox():
obs = spat.geodataframe("bounding_box", [-55, 68, -48, 71])
geom = [Polygon(list(zip([-55, -55, -48, -48, -55], [68, 71, 71, 68, 68])))]
obs = spat.geodataframe("bounding_box", [-55.0, 68.0, -48.0, 71.0])
geom = [
Polygon(
list(
zip([-55.0, -55.0, -48.0, -48.0, -55.0], [68.0, 71.0, 71.0, 68.0, 68.0])
)
)
]
exp = gpd.GeoDataFrame(geometry=geom)

# make sure there is only one geometry before comparing them
Expand All @@ -408,7 +420,9 @@ def test_gdf_from_multi_bbox():


def test_gdf_from_polygon():
polygon = Polygon(list(zip([-55, -55, -48, -48, -55], [68, 71, 71, 68, 68])))
polygon = Polygon(
list(zip([-55.0, -55.0, -48.0, -48.0, -55.0], [68.0, 71.0, 71.0, 68.0, 68.0]))
)
obs = spat.geodataframe("polygon", polygon)
exp = gpd.GeoDataFrame(geometry=[polygon])

Expand Down Expand Up @@ -492,7 +506,7 @@ def test_bad_extent_type_input():
r"Your spatial extent type (polybox) is not an accepted input and a geodataframe cannot be constructed"
)
with pytest.raises(TypeError, match=ermsg):
spat.geodataframe("polybox", [1, 2, 3, 4])
spat.geodataframe("polybox", [1.0, 2.0, 3.0, 4.0])


# ###################### END GEOM FILE INPUT TESTS ####################################################################
Expand Down Expand Up @@ -567,14 +581,14 @@ def test_bbox_fmt():
def test_fmt_for_cmr_fails_unknown_extent_type():
bbox = spat.Spatial([-55, 68, -48, 71])
bbox._ext_type = "Unknown_user_override"
with pytest.raises(icepyx.core.exceptions.ExhaustiveTypeGuardException):
with pytest.raises(ipx.core.exceptions.ExhaustiveTypeGuardException):
bbox.fmt_for_CMR()


def test_fmt_for_egi_fails_unknown_extent_type():
bbox = spat.Spatial([-55, 68, -48, 71])
bbox._ext_type = "Unknown_user_override"
with pytest.raises(icepyx.core.exceptions.ExhaustiveTypeGuardException):
with pytest.raises(ipx.core.exceptions.ExhaustiveTypeGuardException):
bbox.fmt_for_EGI()


Expand Down
1 change: 0 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ exclude = [
ignore = [
"icepyx/quest/*",
"icepyx/core/auth.py",
# "icepyx/core/is2ref.py",
"icepyx/core/read.py",
"icepyx/core/variables.py",
"icepyx/core/visualization.py",
Expand Down

0 comments on commit 0b2f704

Please sign in to comment.