From da9bfc3cb0f11b7c0ed7069507e4049eeb5d7bc8 Mon Sep 17 00:00:00 2001 From: Matic Lubej Date: Tue, 29 Aug 2023 16:58:17 +0200 Subject: [PATCH 01/10] remove compress_level parameter from usable functions --- eolearn/core/core_tasks.py | 4 ---- eolearn/core/eodata.py | 4 ---- eolearn/core/eodata_io.py | 10 +++++----- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/eolearn/core/core_tasks.py b/eolearn/core/core_tasks.py index 08da4aa9..100bb31e 100644 --- a/eolearn/core/core_tasks.py +++ b/eolearn/core/core_tasks.py @@ -102,7 +102,6 @@ def __init__( config: SHConfig | None = None, features: FeaturesSpecification = ..., overwrite_permission: OverwritePermission = OverwritePermission.ADD_ONLY, - compress_level: int = 1, *, save_timestamps: bool | Literal["auto"] = "auto", use_zarr: bool = False, @@ -116,7 +115,6 @@ def __init__( :param features: A collection of features types specifying features of which type will be saved. By default, all features will be saved. :param overwrite_permission: A level of permission for overwriting an existing EOPatch - :param compress_level: A level of data compression and can be specified with an integer from 0 (no compression) to 9 (highest compression). :save_timestamps: Whether to save the timestamps of the EOPatch. With the `"auto"` setting timestamps are saved if `features=...` or if other temporal features are being saved. @@ -127,7 +125,6 @@ def __init__( """ self.features = features self.overwrite_permission = overwrite_permission - self.compress_level = compress_level self.use_zarr = use_zarr self.temporal_selection = temporal_selection self.save_timestamps = save_timestamps @@ -155,7 +152,6 @@ def execute( filesystem=self.filesystem, features=self.features, overwrite_permission=self.overwrite_permission, - compress_level=self.compress_level, save_timestamps=self.save_timestamps, use_zarr=self.use_zarr, temporal_selection=temporal_selection, diff --git a/eolearn/core/eodata.py b/eolearn/core/eodata.py index fb8819b7..79b08033 100644 --- a/eolearn/core/eodata.py +++ b/eolearn/core/eodata.py @@ -604,7 +604,6 @@ def save( path: str, features: FeaturesSpecification = ..., overwrite_permission: OverwritePermission = OverwritePermission.ADD_ONLY, - compress_level: int = 1, filesystem: FS | None = None, *, save_timestamps: bool | Literal["auto"] = "auto", @@ -617,8 +616,6 @@ def save( :param features: A collection of features types specifying features of which type will be saved. By default, all features will be saved. :param overwrite_permission: A level of permission for overwriting an existing EOPatch - :param compress_level: A level of data compression and can be specified with an integer from 0 (no compression) - to 9 (highest compression). :param filesystem: An existing filesystem object. If not given it will be initialized according to the `path` parameter. :save_timestamps: Whether to save the timestamps of the EOPatch. With the `"auto"` setting timestamps are saved @@ -637,7 +634,6 @@ def save( filesystem, path, features=features, - compress_level=compress_level, overwrite_permission=OverwritePermission(overwrite_permission), save_timestamps=save_timestamps, use_zarr=use_zarr, diff --git a/eolearn/core/eodata_io.py b/eolearn/core/eodata_io.py index bcc0408f..64c5dcee 100644 --- a/eolearn/core/eodata_io.py +++ b/eolearn/core/eodata_io.py @@ -116,7 +116,6 @@ def save_eopatch( features: FeaturesSpecification, save_timestamps: bool | Literal["auto"], overwrite_permission: OverwritePermission, - compress_level: int, use_zarr: bool, temporal_selection: None | slice | list[int] | Literal["infer"], ) -> None: @@ -144,7 +143,6 @@ def save_eopatch( patch_location=patch_location, filesystem=filesystem, use_zarr=use_zarr, - compress_level=compress_level, save_timestamps=save_timestamps, temporal_selection=_infer_temporal_selection(temporal_selection, filesystem, file_information, eopatch), ) @@ -409,9 +407,11 @@ def get_filesystem_data_info( if object_name == "timestamp": warnings.warn( - f"EOPatch at {patch_location} contains the deprecated naming `timestamp` for the `timestamps`" - " feature. The old name will no longer be valid in the future. You can re-save the `EOPatch` to" - " update it.", + ( + f"EOPatch at {patch_location} contains the deprecated naming `timestamp` for the `timestamps`" + " feature. The old name will no longer be valid in the future. You can re-save the `EOPatch` to" + " update it." + ), category=EODeprecationWarning, stacklevel=2, ) From 1615804924e99873b1b59d5a90c8e5e4eebe32ba Mon Sep 17 00:00:00 2001 From: Matic Lubej Date: Tue, 29 Aug 2023 17:20:48 +0200 Subject: [PATCH 02/10] update savers, hard-code compression values --- eolearn/core/eodata_io.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/eolearn/core/eodata_io.py b/eolearn/core/eodata_io.py index 64c5dcee..afeac840 100644 --- a/eolearn/core/eodata_io.py +++ b/eolearn/core/eodata_io.py @@ -201,7 +201,6 @@ def _yield_savers( features: Features, patch_location: str, filesystem: FS, - compress_level: int, save_timestamps: bool, use_zarr: bool, temporal_selection: TemporalSelection, @@ -211,20 +210,32 @@ def _yield_savers( if eopatch.bbox is not None: # remove after BBox is never None bbox: BBox = eopatch.bbox # mypy has problems - yield partial(FeatureIOBBox.save, bbox, filesystem, get_file_path(BBOX_FILENAME), compress_level) + yield partial( + FeatureIOBBox.save, + data=bbox, + filesystem=filesystem, + feature_path=get_file_path(BBOX_FILENAME), + compress_level=0, + ) if eopatch.timestamps is not None and save_timestamps: path = get_file_path(TIMESTAMPS_FILENAME) - yield partial(FeatureIOTimestamps.save, eopatch.timestamps, filesystem, path, compress_level) + yield partial( + FeatureIOTimestamps.save, + data=eopatch.timestamps, + filesystem=filesystem, + feature_path=path, + compress_level=0, + ) for ftype, fname in features: io_constructor = _get_feature_io_constructor(ftype, use_zarr) feature_saver = partial( io_constructor.save, - compress_level=compress_level, - filesystem=filesystem, data=eopatch[ftype, fname], + filesystem=filesystem, feature_path=get_file_path(ftype.value, fname), + compress_level=1, ) if ftype.is_temporal() and issubclass(io_constructor, FeatureIOZarr): From 4b321c56fc645936641778c3bd58bcff96945e1e Mon Sep 17 00:00:00 2001 From: Matic Lubej Date: Tue, 29 Aug 2023 17:21:00 +0200 Subject: [PATCH 03/10] remove compression param from tests --- tests/core/test_eodata_io.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/core/test_eodata_io.py b/tests/core/test_eodata_io.py index 9b0cdaaa..e493a838 100644 --- a/tests/core/test_eodata_io.py +++ b/tests/core/test_eodata_io.py @@ -338,7 +338,7 @@ def test_save_and_load_tasks(eopatch, fs_loader, use_zarr: bool): with fs_loader() as temp_fs: temp_fs.makedir(folder) - save_task = SaveTask(folder, filesystem=temp_fs, compress_level=9, use_zarr=use_zarr) + save_task = SaveTask(folder, filesystem=temp_fs, use_zarr=use_zarr) load_task = LoadTask(folder, filesystem=temp_fs, lazy_loading=False) saved_eop = save_task(eopatch, eopatch_folder=patch_folder) @@ -411,10 +411,10 @@ def test_cleanup_different_compression(fs_loader, eopatch): temp_fs.makedir(folder) save_compressed_task = SaveTask( - folder, filesystem=temp_fs, compress_level=9, overwrite_permission="OVERWRITE_FEATURES" + folder, filesystem=temp_fs, overwrite_permission="OVERWRITE_FEATURES" ) save_noncompressed_task = SaveTask( - folder, filesystem=temp_fs, compress_level=0, overwrite_permission="OVERWRITE_FEATURES" + folder, filesystem=temp_fs, overwrite_permission="OVERWRITE_FEATURES" ) bbox_path = fs.path.join(folder, patch_folder, "bbox.geojson") compressed_bbox_path = bbox_path + ".gz" @@ -445,10 +445,10 @@ def test_cleanup_different_compression_zarr(eopatch): temp_fs.makedir(folder) save_compressed_task = SaveTask( - folder, filesystem=temp_fs, compress_level=9, overwrite_permission="OVERWRITE_FEATURES", use_zarr=True + folder, filesystem=temp_fs, overwrite_permission="OVERWRITE_FEATURES", use_zarr=True ) save_noncompressed_task = SaveTask( - folder, filesystem=temp_fs, compress_level=0, overwrite_permission="OVERWRITE_FEATURES", use_zarr=True + folder, filesystem=temp_fs, overwrite_permission="OVERWRITE_FEATURES", use_zarr=True ) bbox_path = fs.path.join(folder, patch_folder, "bbox.geojson") compressed_bbox_path = bbox_path + ".gz" @@ -520,17 +520,16 @@ def test_lazy_loading_plus_overwrite_patch(fs_loader, folder_name, eopatch, use_ (FeatureIOTimestamps, [datetime.datetime(2017, 1, 1, 10, 4, 7), datetime.datetime(2017, 1, 4, 10, 14, 5)]), ], ) -@pytest.mark.parametrize("compress_level", [0, 1]) -def test_feature_io(constructor: type[FeatureIO], data: Any, compress_level: int) -> None: +def test_feature_io(constructor: type[FeatureIO], data: Any) -> None: """ Tests verifying that FeatureIO subclasses correctly save, load, and lazy-load data. Test cases do not include subfolders, because subfolder management is currently done by the `save_eopatch` function. """ - file_extension = constructor.get_file_extension(compress_level=compress_level) + file_extension = constructor.get_file_extension(compress_level=1) file_name = "name" with TempFS("testing_file_sistem") as temp_fs: feat_io = constructor(file_name + file_extension, filesystem=temp_fs) - constructor.save(data, temp_fs, file_name, compress_level) + constructor.save(data, temp_fs, file_name) loaded_data = feat_io.load() assert_feature_data_equal(loaded_data, data) @@ -739,7 +738,7 @@ def test_partial_temporal_saving_fails(eopatch: EOPatch): @pytest.mark.parametrize("patch_location", [".", "patch-folder", "some/long/path"]) def test_old_style_meta_info(patch_location): with TempFS() as temp_fs: - EOPatch(bbox=DUMMY_BBOX).save(path=patch_location, filesystem=temp_fs, compress_level=0) + EOPatch(bbox=DUMMY_BBOX).save(path=patch_location, filesystem=temp_fs) meta_info = {"this": ["list"], "something": "else"} with temp_fs.open(f"{patch_location}/meta_info.json", "w") as old_style_file: json.dump(meta_info, old_style_file) @@ -749,7 +748,7 @@ def test_old_style_meta_info(patch_location): assert dict(loaded_patch.meta_info.items()) == meta_info loaded_patch.meta_info = {"beep": "boop"} - loaded_patch.save(path=patch_location, filesystem=temp_fs, compress_level=0) + loaded_patch.save(path=patch_location, filesystem=temp_fs) assert not temp_fs.exists(f"{patch_location}/meta_info.json") assert temp_fs.exists(f"{patch_location}/meta_info/beep.json") From a051e5a9b7006bc36f97e3fa2d6fe11551a9b388 Mon Sep 17 00:00:00 2001 From: Matic Lubej Date: Tue, 29 Aug 2023 17:45:44 +0200 Subject: [PATCH 04/10] fix compression-related tests --- tests/core/test_eodata_io.py | 89 +++++------------------------------- 1 file changed, 11 insertions(+), 78 deletions(-) diff --git a/tests/core/test_eodata_io.py b/tests/core/test_eodata_io.py index e493a838..52788765 100644 --- a/tests/core/test_eodata_io.py +++ b/tests/core/test_eodata_io.py @@ -7,7 +7,6 @@ from __future__ import annotations import datetime -import json import os import sys import warnings @@ -104,7 +103,7 @@ def test_saving_in_empty_folder(eopatch, fs_loader, use_zarr: bool): subfolder = "new-subfolder" eopatch.save("new-subfolder", filesystem=temp_fs, use_zarr=use_zarr) - assert temp_fs.exists(f"/{subfolder}/bbox.geojson.gz") + assert temp_fs.exists(f"/{subfolder}/bbox.geojson") @mock_s3 @@ -218,7 +217,7 @@ def test_save_add_only_features(eopatch, fs_loader, use_zarr: bool): def test_bbox_always_saved(eopatch, fs_loader): with fs_loader() as temp_fs: eopatch.save("/", filesystem=temp_fs, features=[FeatureType.DATA]) - assert temp_fs.exists("/bbox.geojson.gz") + assert temp_fs.exists("/bbox.geojson") @mock_s3 @@ -239,7 +238,7 @@ def test_bbox_always_saved(eopatch, fs_loader): def test_save_timestamps(eopatch, fs_loader, save_timestamps, features, should_save): with fs_loader() as temp_fs: eopatch.save("/", filesystem=temp_fs, features=features, save_timestamps=save_timestamps) - assert temp_fs.exists("/timestamps.json.gz") == should_save + assert temp_fs.exists("/timestamps.json") == should_save def test_auto_save_load_timestamps(eopatch): @@ -247,7 +246,7 @@ def test_auto_save_load_timestamps(eopatch): test_patch = EOPatch(bbox=eopatch.bbox, timestamps=eopatch.timestamps) # no temporal stuff with TempFS() as temp_fs: test_patch.save("/", filesystem=temp_fs) - assert temp_fs.exists("/timestamps.json.gz") + assert temp_fs.exists("/timestamps.json") assert EOPatch.load("/", filesystem=temp_fs).timestamps is not None @@ -342,7 +341,7 @@ def test_save_and_load_tasks(eopatch, fs_loader, use_zarr: bool): load_task = LoadTask(folder, filesystem=temp_fs, lazy_loading=False) saved_eop = save_task(eopatch, eopatch_folder=patch_folder) - bbox_path = fs.path.join(folder, patch_folder, "bbox.geojson.gz") + bbox_path = fs.path.join(folder, patch_folder, "bbox.geojson") assert temp_fs.exists(bbox_path) assert saved_eop == eopatch @@ -402,73 +401,6 @@ def test_nonexistent_location(fs_loader, use_zarr: bool): assert os.path.exists(full_path) -@mock_s3 -@pytest.mark.parametrize("fs_loader", FS_LOADERS) -def test_cleanup_different_compression(fs_loader, eopatch): - folder = "foo-folder" - patch_folder = "patch-folder" - with fs_loader() as temp_fs: - temp_fs.makedir(folder) - - save_compressed_task = SaveTask( - folder, filesystem=temp_fs, overwrite_permission="OVERWRITE_FEATURES" - ) - save_noncompressed_task = SaveTask( - folder, filesystem=temp_fs, overwrite_permission="OVERWRITE_FEATURES" - ) - bbox_path = fs.path.join(folder, patch_folder, "bbox.geojson") - compressed_bbox_path = bbox_path + ".gz" - mask_timeless_path = fs.path.join(folder, patch_folder, "mask_timeless", "mask.npy") - compressed_mask_timeless_path = mask_timeless_path + ".gz" - - save_compressed_task(eopatch, eopatch_folder=patch_folder) - save_noncompressed_task(eopatch, eopatch_folder=patch_folder) - assert temp_fs.exists(bbox_path) - assert temp_fs.exists(mask_timeless_path) - assert not temp_fs.exists(compressed_bbox_path) - assert not temp_fs.exists(compressed_mask_timeless_path) - - save_compressed_task(eopatch, eopatch_folder=patch_folder) - assert not temp_fs.exists(bbox_path) - assert not temp_fs.exists(mask_timeless_path) - assert temp_fs.exists(compressed_bbox_path) - assert temp_fs.exists(compressed_mask_timeless_path) - - -def test_cleanup_different_compression_zarr(eopatch): - # zarr and moto dont work atm anyway - # slightly different than regular one, since zarr does not use gzip to compress itself - _skip_when_appropriate(None, True) - folder = "foo-folder" - patch_folder = "patch-folder" - with TempFS() as temp_fs: - temp_fs.makedir(folder) - - save_compressed_task = SaveTask( - folder, filesystem=temp_fs, overwrite_permission="OVERWRITE_FEATURES", use_zarr=True - ) - save_noncompressed_task = SaveTask( - folder, filesystem=temp_fs, overwrite_permission="OVERWRITE_FEATURES", use_zarr=True - ) - bbox_path = fs.path.join(folder, patch_folder, "bbox.geojson") - compressed_bbox_path = bbox_path + ".gz" - mask_timeless_path = fs.path.join(folder, patch_folder, "mask_timeless", "mask.zarr") - wrong_compressed_mask_timeless_path = mask_timeless_path + ".gz" - - save_compressed_task(eopatch, eopatch_folder=patch_folder) - save_noncompressed_task(eopatch, eopatch_folder=patch_folder) - assert temp_fs.exists(bbox_path) - assert temp_fs.exists(mask_timeless_path) - assert not temp_fs.exists(compressed_bbox_path) - assert not temp_fs.exists(wrong_compressed_mask_timeless_path) - - save_compressed_task(eopatch, eopatch_folder=patch_folder) - assert not temp_fs.exists(bbox_path) - assert temp_fs.exists(mask_timeless_path) - assert temp_fs.exists(compressed_bbox_path) - assert not temp_fs.exists(wrong_compressed_mask_timeless_path) - - @mock_s3 @pytest.mark.parametrize("fs_loader", FS_LOADERS) @pytest.mark.parametrize("use_zarr", [True, False]) @@ -529,7 +461,7 @@ def test_feature_io(constructor: type[FeatureIO], data: Any) -> None: file_name = "name" with TempFS("testing_file_sistem") as temp_fs: feat_io = constructor(file_name + file_extension, filesystem=temp_fs) - constructor.save(data, temp_fs, file_name) + constructor.save(data, temp_fs, file_name, compress_level=1) loaded_data = feat_io.load() assert_feature_data_equal(loaded_data, data) @@ -740,8 +672,9 @@ def test_old_style_meta_info(patch_location): with TempFS() as temp_fs: EOPatch(bbox=DUMMY_BBOX).save(path=patch_location, filesystem=temp_fs) meta_info = {"this": ["list"], "something": "else"} - with temp_fs.open(f"{patch_location}/meta_info.json", "w") as old_style_file: - json.dump(meta_info, old_style_file) + file_name, file_extension = f"{patch_location}/meta_info", ".json.gz" + old_style_io = FeatureIOJson(file_name + file_extension, filesystem=temp_fs) + old_style_io.save(meta_info, temp_fs, file_name, compress_level=1) with pytest.warns(EODeprecationWarning): loaded_patch = EOPatch.load(path=patch_location, filesystem=temp_fs) @@ -749,8 +682,8 @@ def test_old_style_meta_info(patch_location): loaded_patch.meta_info = {"beep": "boop"} loaded_patch.save(path=patch_location, filesystem=temp_fs) - assert not temp_fs.exists(f"{patch_location}/meta_info.json") - assert temp_fs.exists(f"{patch_location}/meta_info/beep.json") + assert not temp_fs.exists(f"{patch_location}/meta_info.json.gz") + assert temp_fs.exists(f"{patch_location}/meta_info/beep.json.gz") loaded_patch = EOPatch.load(path=patch_location, filesystem=temp_fs) assert dict(loaded_patch.meta_info.items()) == {"beep": "boop"} From 822f940323c3ff706b18bf7611adcfd45448309e Mon Sep 17 00:00:00 2001 From: Matic Lubej Date: Tue, 29 Aug 2023 17:51:09 +0200 Subject: [PATCH 05/10] formatting + linters --- eolearn/core/eodata_io.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/eolearn/core/eodata_io.py b/eolearn/core/eodata_io.py index afeac840..a0e980a8 100644 --- a/eolearn/core/eodata_io.py +++ b/eolearn/core/eodata_io.py @@ -418,11 +418,9 @@ def get_filesystem_data_info( if object_name == "timestamp": warnings.warn( - ( - f"EOPatch at {patch_location} contains the deprecated naming `timestamp` for the `timestamps`" - " feature. The old name will no longer be valid in the future. You can re-save the `EOPatch` to" - " update it." - ), + f"EOPatch at {patch_location} contains the deprecated naming `timestamp` for the `timestamps`" + " feature. The old name will no longer be valid in the future. You can re-save the `EOPatch` to" + " update it.", category=EODeprecationWarning, stacklevel=2, ) From 94b2b141703afba4d24fe12e8ca27d093ede7eb3 Mon Sep 17 00:00:00 2001 From: Matic Lubej Date: Wed, 30 Aug 2023 15:53:07 +0200 Subject: [PATCH 06/10] add deprecation warnings when `compress_level` param is used --- eolearn/core/core_tasks.py | 9 +++++++++ eolearn/core/eodata.py | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/eolearn/core/core_tasks.py b/eolearn/core/core_tasks.py index 100bb31e..98334e85 100644 --- a/eolearn/core/core_tasks.py +++ b/eolearn/core/core_tasks.py @@ -10,6 +10,7 @@ import copy import datetime as dt +import warnings from abc import ABCMeta from typing import Any, Callable, Iterable, Literal, Tuple, Union, cast @@ -106,6 +107,7 @@ def __init__( save_timestamps: bool | Literal["auto"] = "auto", use_zarr: bool = False, temporal_selection: None | slice | list[int] | Literal["infer"] = None, + compress_level: int | None = None, ): """ :param path: root path where all EOPatches are saved @@ -130,6 +132,13 @@ def __init__( self.save_timestamps = save_timestamps super().__init__(path, filesystem=filesystem, create=True, config=config) + if compress_level is not None: + warnings.warn( + "The `compress_level` parameter has been deprecated, data is now compressed by default.", + category=EODeprecationWarning, + stacklevel=2, + ) + def execute( self, eopatch: EOPatch, diff --git a/eolearn/core/eodata.py b/eolearn/core/eodata.py index 79b08033..0a1b8549 100644 --- a/eolearn/core/eodata.py +++ b/eolearn/core/eodata.py @@ -609,6 +609,7 @@ def save( save_timestamps: bool | Literal["auto"] = "auto", use_zarr: bool = False, temporal_selection: None | slice | list[int] | Literal["infer"] = None, + compress_level: int | None = None, ) -> None: """Method to save an EOPatch from memory to a storage. @@ -629,6 +630,13 @@ def save( filesystem = get_filesystem(path, create=True) path = "/" + if compress_level is not None: + warnings.warn( + "The `compress_level` parameter has been deprecated, data is now compressed by default.", + category=EODeprecationWarning, + stacklevel=2, + ) + save_eopatch( self, filesystem, From d4286f5414d9b602d9e7a6993db2e1c868a8afd4 Mon Sep 17 00:00:00 2001 From: Matic Lubej Date: Thu, 31 Aug 2023 11:32:31 +0200 Subject: [PATCH 07/10] add test for compression deprecation --- tests/core/test_eodata_io.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/core/test_eodata_io.py b/tests/core/test_eodata_io.py index 52788765..79ee98ca 100644 --- a/tests/core/test_eodata_io.py +++ b/tests/core/test_eodata_io.py @@ -327,6 +327,19 @@ def test_overwrite_failure(fs_loader, use_zarr: bool): ) +@mock_s3 +@pytest.mark.parametrize("fs_loader", FS_LOADERS) +@pytest.mark.parametrize("compress_level", [0, 1]) +def test_compression_deprecation(eopatch, fs_loader, compress_level: int | None): + folder = "foo-folder" + + with fs_loader() as temp_fs, pytest.warns(EODeprecationWarning): + SaveTask(folder, filesystem=temp_fs, compress_level=compress_level) + + with fs_loader() as temp_fs, pytest.warns(EODeprecationWarning): + eopatch.save(folder, filesystem=temp_fs, compress_level=compress_level) + + @mock_s3 @pytest.mark.parametrize("fs_loader", FS_LOADERS) @pytest.mark.parametrize("use_zarr", [True, False]) From ee330882b291c4413d5e57b3ca31d288b6b1619b Mon Sep 17 00:00:00 2001 From: Matic Lubej Date: Thu, 31 Aug 2023 11:33:09 +0200 Subject: [PATCH 08/10] add test for compression cleanup and backwards compatibility --- tests/core/test_eodata_io.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/core/test_eodata_io.py b/tests/core/test_eodata_io.py index 79ee98ca..7100291d 100644 --- a/tests/core/test_eodata_io.py +++ b/tests/core/test_eodata_io.py @@ -414,6 +414,31 @@ def test_nonexistent_location(fs_loader, use_zarr: bool): assert os.path.exists(full_path) +@mock_s3 +@pytest.mark.parametrize("fs_loader", FS_LOADERS) +def test_cleanup_different_compression(fs_loader, eopatch): + folder = "foo-folder" + patch_folder = "patch-folder" + with fs_loader() as temp_fs: + save_task = SaveTask(folder, filesystem=temp_fs, overwrite_permission="OVERWRITE_FEATURES") + bbox_path = fs.path.join(folder, patch_folder, "bbox.geojson") + timestamps_path = fs.path.join(folder, patch_folder, "timestamps.json") + + # need to manually save uncompressed feature + ftype, fname = (FeatureType.MASK_TIMELESS, "mask") + ftype_path = fs.path.join(folder, patch_folder, ftype.value) + temp_fs.makedirs(ftype_path) + feature_io = FeatureIONumpy(os.path.join(ftype_path, fname + ".npy"), filesystem=temp_fs) + feature_io.save(eopatch[(ftype, fname)], temp_fs, os.path.join(ftype_path, fname), compress_level=0) + + # re-save compressed and check cleanup + save_task(eopatch, eopatch_folder=patch_folder) + assert temp_fs.exists(bbox_path) + assert temp_fs.exists(timestamps_path) + assert temp_fs.exists(os.path.join(ftype_path, fname) + ".npy.gz") + assert not temp_fs.exists(os.path.join(ftype_path, fname) + ".npy") + + @mock_s3 @pytest.mark.parametrize("fs_loader", FS_LOADERS) @pytest.mark.parametrize("use_zarr", [True, False]) From 0c81cb190fbbae7d6c692a66b6401e250cfe402a Mon Sep 17 00:00:00 2001 From: Matic Lubej Date: Thu, 31 Aug 2023 14:19:01 +0200 Subject: [PATCH 09/10] review comments --- tests/core/test_eodata_io.py | 43 +++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/tests/core/test_eodata_io.py b/tests/core/test_eodata_io.py index 7100291d..531d670c 100644 --- a/tests/core/test_eodata_io.py +++ b/tests/core/test_eodata_io.py @@ -333,11 +333,17 @@ def test_overwrite_failure(fs_loader, use_zarr: bool): def test_compression_deprecation(eopatch, fs_loader, compress_level: int | None): folder = "foo-folder" - with fs_loader() as temp_fs, pytest.warns(EODeprecationWarning): - SaveTask(folder, filesystem=temp_fs, compress_level=compress_level) + with warnings.catch_warnings(): # make warnings errors + warnings.simplefilter("error") - with fs_loader() as temp_fs, pytest.warns(EODeprecationWarning): - eopatch.save(folder, filesystem=temp_fs, compress_level=compress_level) + with fs_loader() as temp_fs: + SaveTask(folder, filesystem=temp_fs) + + with fs_loader() as temp_fs, pytest.warns(EODeprecationWarning): + SaveTask(folder, filesystem=temp_fs, compress_level=compress_level) + + with fs_loader() as temp_fs, pytest.warns(EODeprecationWarning): + eopatch.save(folder, filesystem=temp_fs, compress_level=compress_level) @mock_s3 @@ -420,23 +426,29 @@ def test_cleanup_different_compression(fs_loader, eopatch): folder = "foo-folder" patch_folder = "patch-folder" with fs_loader() as temp_fs: + temp_fs.makedirs(fs.path.join(folder, patch_folder)) save_task = SaveTask(folder, filesystem=temp_fs, overwrite_permission="OVERWRITE_FEATURES") + + # need to manually save uncompressed features bbox_path = fs.path.join(folder, patch_folder, "bbox.geojson") + FeatureIOBBox.save(eopatch.bbox, temp_fs, bbox_path, compress_level=0) + timestamps_path = fs.path.join(folder, patch_folder, "timestamps.json") + FeatureIOTimestamps.save(eopatch.timestamps, temp_fs, timestamps_path, compress_level=0) - # need to manually save uncompressed feature ftype, fname = (FeatureType.MASK_TIMELESS, "mask") - ftype_path = fs.path.join(folder, patch_folder, ftype.value) - temp_fs.makedirs(ftype_path) - feature_io = FeatureIONumpy(os.path.join(ftype_path, fname + ".npy"), filesystem=temp_fs) - feature_io.save(eopatch[(ftype, fname)], temp_fs, os.path.join(ftype_path, fname), compress_level=0) + temp_fs.makedir(fs.path.join(folder, patch_folder, ftype.value)) + mask_timeless_path = fs.path.join(folder, patch_folder, ftype.value, f"{fname}.npy") + FeatureIONumpy.save(eopatch[(ftype, fname)], temp_fs, mask_timeless_path, compress_level=0) - # re-save compressed and check cleanup + # re-save compressed and check cleanup, bbox and timestamps are not compressed save_task(eopatch, eopatch_folder=patch_folder) assert temp_fs.exists(bbox_path) assert temp_fs.exists(timestamps_path) - assert temp_fs.exists(os.path.join(ftype_path, fname) + ".npy.gz") - assert not temp_fs.exists(os.path.join(ftype_path, fname) + ".npy") + assert temp_fs.exists(mask_timeless_path + ".gz") + assert not temp_fs.exists(bbox_path + ".gz") + assert not temp_fs.exists(timestamps_path + ".gz") + assert not temp_fs.exists(mask_timeless_path) @mock_s3 @@ -490,16 +502,17 @@ def test_lazy_loading_plus_overwrite_patch(fs_loader, folder_name, eopatch, use_ (FeatureIOTimestamps, [datetime.datetime(2017, 1, 1, 10, 4, 7), datetime.datetime(2017, 1, 4, 10, 14, 5)]), ], ) -def test_feature_io(constructor: type[FeatureIO], data: Any) -> None: +@pytest.mark.parametrize("compress_level", [0, 1]) +def test_feature_io(constructor: type[FeatureIO], data: Any, compress_level: int) -> None: """ Tests verifying that FeatureIO subclasses correctly save, load, and lazy-load data. Test cases do not include subfolders, because subfolder management is currently done by the `save_eopatch` function. """ - file_extension = constructor.get_file_extension(compress_level=1) + file_extension = constructor.get_file_extension(compress_level=compress_level) file_name = "name" with TempFS("testing_file_sistem") as temp_fs: feat_io = constructor(file_name + file_extension, filesystem=temp_fs) - constructor.save(data, temp_fs, file_name, compress_level=1) + constructor.save(data, temp_fs, file_name, compress_level=compress_level) loaded_data = feat_io.load() assert_feature_data_equal(loaded_data, data) From 07a3343a3b71a897f0e720cbca38c2686de50738 Mon Sep 17 00:00:00 2001 From: Matic Lubej Date: Thu, 31 Aug 2023 14:31:42 +0200 Subject: [PATCH 10/10] add backwards compatibility test for loading uncompressed data --- tests/core/test_eodata_io.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/core/test_eodata_io.py b/tests/core/test_eodata_io.py index 531d670c..e9692cd9 100644 --- a/tests/core/test_eodata_io.py +++ b/tests/core/test_eodata_io.py @@ -441,6 +441,9 @@ def test_cleanup_different_compression(fs_loader, eopatch): mask_timeless_path = fs.path.join(folder, patch_folder, ftype.value, f"{fname}.npy") FeatureIONumpy.save(eopatch[(ftype, fname)], temp_fs, mask_timeless_path, compress_level=0) + # test EOPatch load of uncompressed features + EOPatch.load(fs.path.join(folder, patch_folder), filesystem=temp_fs) + # re-save compressed and check cleanup, bbox and timestamps are not compressed save_task(eopatch, eopatch_folder=patch_folder) assert temp_fs.exists(bbox_path)