Skip to content

Commit

Permalink
Small tweeks to docstrings, remove use of default mutable argument, f…
Browse files Browse the repository at this point in the history
…ix minor security problem.
  • Loading branch information
AdamJCrawford committed Oct 30, 2023
1 parent c540b96 commit da77bdf
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 33 deletions.
8 changes: 4 additions & 4 deletions eedl/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ def _get_fiona_args(polygon_path: Union[str, Path]) -> Dict[str, Union[str, Path

def safe_fiona_open(features_path: Union[str, Path], **extra_kwargs) -> fiona.Collection:
"""
Handles opening things in fiona in a way that is safe, even for geodatabases where we need
to open the geodatabase itself and specify a layer. The caller is responsible for
ensuring the features are closed (e.g. a try/finally block with a call to features.close()
in the finally block should immediately follow calling this function.
Handles opening things in fiona in a way that is safe, even for geodatabases where we need
to open the geodatabase itself and specify a layer. The caller is responsible for
ensuring the features are closed (e.g. a try/finally block with a call to features.close())
in the finally block should immediately follow calling this function.
:param features_path: A Path object or string path to open with fiona
:param extra_kwargs: Keyword arguments to directly pass through to fiona. Helpful when trying to filter features, etc
:return:
Expand Down
25 changes: 17 additions & 8 deletions eedl/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,20 @@ def __init__(self, **kwargs):

def _single_item_extract(self, image, task_registry, zonal_features, aoi_attr, ee_geom, image_date, aoi_download_folder):
"""
This looks a bit silly here, but we need to construct this here so that we have access
to this method's variables since we can't pass them in and it can't be a class function.
:param image:
:param state:
:return:
This looks a bit silly here, but we need to construct this here so that we have access
to this method's variables since we can't pass them in and it can't be a class function.
Args:
image:
task_registry:
zonal_features:
aoi_attr:
ee_geom:
image_date:
aoi_download_folder:
Returns:
None
"""

export_image = EEDLImage(
Expand Down Expand Up @@ -188,9 +197,9 @@ def _get_and_filter_collection(self):

def mosaic_by_date(image_collection):
"""
Adapted to Python from code found via https://gis.stackexchange.com/a/343453/1955
:param image_collection: An image collection
:return: ee.ImageCollection
Adapted to Python from code found via https://gis.stackexchange.com/a/343453/1955
:param image_collection: An image collection
:return: ee.ImageCollection
"""
image_list = image_collection.toList(image_collection.size())

Expand Down
35 changes: 21 additions & 14 deletions eedl/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def __init__(self) -> None:
self.log_file: Optional[io.TextIOWrapper] = None # the open log file handle
self.raise_errors: bool = True

def add(self, image: ee.image.Image) -> None:
def add(self, image: EEDLImage) -> None:
"""
Adds an Earth Engine image to the list of Earth Engine images.
Expand All @@ -94,7 +94,7 @@ def add(self, image: ee.image.Image) -> None:
self.images.append(image)

@property
def incomplete_tasks(self) -> List[ee.image.Image]:
def incomplete_tasks(self) -> List[EEDLImage]:
"""
List of Earth Engine images that have not been completed yet.
Expand All @@ -108,7 +108,7 @@ def incomplete_tasks(self) -> List[ee.image.Image]:
return [image for image in self.images if image.last_task_status['state'] in self.INCOMPLETE_STATUSES]

@property
def complete_tasks(self) -> List[ee.image.Image]:
def complete_tasks(self) -> List[EEDLImage]:
"""
List of Earth Engine images.
Expand All @@ -118,7 +118,7 @@ def complete_tasks(self) -> List[ee.image.Image]:
return [image for image in self.images if image.last_task_status['state'] in self.COMPLETE_STATUSES + self.FAILED_STATUSES]

@property
def failed_tasks(self) -> List[ee.image.Image]:
def failed_tasks(self) -> List[EEDLImage]:
"""
List of Earth Engine images that have either been cancelled or that have failed
Expand All @@ -128,7 +128,7 @@ def failed_tasks(self) -> List[ee.image.Image]:
return [image for image in self.images if image.last_task_status['state'] in self.FAILED_STATUSES]

@property
def downloadable_tasks(self) -> List[ee.image.Image]:
def downloadable_tasks(self) -> List[EEDLImage]:
"""
List of Earth Engine images that have not been cancelled or have failed.
Expand Down Expand Up @@ -165,9 +165,12 @@ def setup_log(self, log_file_path: Union[str, Path], mode='a'):

def log_error(self, error_type: str, error_message: str):
"""
:param error_type: Options "ee", "local" to indicate whether it was an error on Earth Engine's side or on
the local processing side
:param error_message: The error message to print to the log file
Args:
error_type (str): Options "ee", "local" to indicate whether it was an error on Earth Engine's side or on the local processing side
error_message (str): The error message to print to the log file
Returns:
None
"""
message = f"{error_type} Error: {error_message}"
date_string = datetime.datetime.now().strftime("%Y-%m-%d %H:%M")
Expand Down Expand Up @@ -387,9 +390,9 @@ def export(self,
(drive_root_folder is None or not os.path.exists(drive_root_folder)):

raise NotADirectoryError("The provided path for the Google Drive export folder is not a valid directory but"
" Drive export was specified. Either change the export type to use Google Cloud"
" and set that up properly (with a bucket, etc), or set the drive_root_folder"
" to a valid folder.")
"Drive export was specified. Either change the export type to use Google Cloud"
"and set that up properly (with a bucket, etc), or set the drive_root_folder"
"to a valid folder.")
elif export_type.lower() == "drive":
if drive_root_folder:
self.drive_root_folder = drive_root_folder
Expand Down Expand Up @@ -544,12 +547,11 @@ def zonal_stats(self,
report_threshold: int = 1000,
write_batch_size: int = 2000,
use_points: bool = False,
inject_constants: dict = dict(),
inject_constants: Optional[dict] = None,
nodata_value: int = -9999,
all_touched: bool = False
) -> None:
"""
Args:
polygons (Union[str, Path]):
keep_fields (tuple[str, ...]):
Expand All @@ -558,10 +560,15 @@ def zonal_stats(self,
Set to None to disable.
write_batch_size (int): How many zones should we store up before writing to the disk? Defaults to 2000.
use_points (bool):
inject_constants(Optional[dict]):
nodata_value (int):
all_touched (bool):
Returns:
None
"""
if inject_constants is None:
inject_constants = dict()

self.zonal_output_filepath = zonal.zonal_stats(
polygons,
Expand All @@ -580,7 +587,7 @@ def zonal_stats(self,

def _check_task_status(self) -> Dict[str, Union[Dict[str, str], bool]]:
"""
Updates the status is it needs to be changed
Updates the status if it needs to be changed
Returns:
Dict[str, Union[Dict[str, str], bool]]: Returns a dictionary of the most up-to-date status and whether that status was changed
Expand Down
5 changes: 2 additions & 3 deletions eedl/mosaic_rasters.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import tempfile
from pathlib import Path
from typing import Sequence, Union

from osgeo import gdal


Expand All @@ -21,7 +20,7 @@ def mosaic_folder(folder_path: Union[str, Path], output_path: Union[str, Path],
"""
tifs = [os.path.join(folder_path, filename) for filename in os.listdir(folder_path) if filename.endswith(".tif") and filename.startswith(prefix)]

if len(tifs) == 1: # If we only got one image back, don't both mosaicking, though this will also skip generating overviews.
if len(tifs) == 1: # If we only got one image back, don't bother mosaicking, though this will also skip generating overviews.
shutil.move(tifs[0], output_path) # Just move the output image to the "mosaic" name, then return.
return

Expand All @@ -44,7 +43,7 @@ def mosaic_rasters(raster_paths: Sequence[Union[str, Path]],
"""

# gdal.SetConfigOption("GTIFF_SRC_SOURCE", "GEOKEYS")
vrt_path = tempfile.mktemp(suffix=".vrt", prefix="mosaic_rasters_")
vrt_path = tempfile.mkstemp(suffix=".vrt", prefix="mosaic_rasters_")

vrt_options = gdal.BuildVRTOptions(resampleAlg='nearest', resolution="highest")
my_vrt = gdal.BuildVRT(vrt_path, raster_paths, options=vrt_options)
Expand Down
6 changes: 2 additions & 4 deletions examples/basic.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import ee
from ee import ImageCollection

# we should change the name of our Image class - it conflicts with the class image in the ee package, and people will
# likely be using both. Let's not cause confusion
from eedl.image import EEDLImage
import eedl
from eedl.image import EEDLImage


def test_simple() -> None:
Expand All @@ -14,7 +12,7 @@ def test_simple() -> None:
# Adam, make sure to set the drive root folder for your own testing - we'll need to fix this, and in the future,
# we can use a Google Cloud bucket for most testing this is clunky - we should make the instantiation of the image be able to take a kwarg that sets the value of image, I think.
image = EEDLImage()
image.export(s2_image, "valley_water_s2_test_image", export_type="Drive", drive_root_folder=r"G:\My Drive", clip=geometry)
image.export(s2_image, "valley_water_s2_test_image", export_type="Drive", clip=geometry, drive_root_folder=r"G:\My Drive")

# We need to make it check and report whether the export on the EE side was successful. This test "passed" because Earth Engine failed and there wasn't anything to download (oops)
# Adam, make sure to set the folder you want results to be downloaded to
Expand Down

0 comments on commit da77bdf

Please sign in to comment.