Skip to content
This repository was archived by the owner on Jun 27, 2025. It is now read-only.

Commit 85bafdc

Browse files
author
yohplala
committed
OPD has now a working 'remove_from_disk()' method.
1 parent d5ddb53 commit 85bafdc

5 files changed

Lines changed: 79 additions & 15 deletions

File tree

oups/store/ordered_parquet_dataset/ordered_parquet_dataset/base.py

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -285,14 +285,6 @@ def __init__(
285285
self._release_lock()
286286
raise
287287

288-
def _release_lock(self):
289-
"""
290-
Release lock with reference counting.
291-
"""
292-
self._lock._ref_count -= 1
293-
if self._lock._ref_count <= 0:
294-
self._lock.unlock(unconditionally=True)
295-
296288
def __del__(self):
297289
"""
298290
Release lock when object is garbage collected.
@@ -411,7 +403,7 @@ def max_file_id(self):
411403
# Get max 'file_id' from 'self.row_group_stats'.
412404
return -1 if self.row_group_stats.empty else int(self.row_group_stats[KEY_FILE_IDS].max())
413405

414-
def remove_from_disk(self, preserve_metadata: bool = False):
406+
def remove_from_disk(self, preserve_metadata: bool = False, release_lock: bool = True) -> None:
415407
"""
416408
Remove all dataset files from disk and update in-memory state.
417409
@@ -420,9 +412,19 @@ def remove_from_disk(self, preserve_metadata: bool = False):
420412
preserve_metadata : bool, default False
421413
If True, keep user metadata accessible but clear row_group_stats.
422414
If False, reset both row_group_stats and key_value_metadata.
415+
release_lock : bool, default True
416+
If True, release the lock after removal. Set to False if you plan
417+
to continue using this OPD instance after removal.
418+
419+
Notes
420+
-----
421+
After calling this method with ``release_lock=True``, the OPD instance
422+
should not be used for file operations, though metadata access remains
423+
available if ``preserve_metadata=True``.
423424
424425
"""
425426
if not self._is_newly_initialized:
427+
self._lock.refresh(unconditionally=True)
426428
# Remove dataset directory and all its contents
427429
remove_dir(self.dirpath)
428430
get_md_filepath(self.dirpath).unlink()
@@ -432,6 +434,8 @@ def remove_from_disk(self, preserve_metadata: bool = False):
432434
self._row_group_stats = DataFrame(columns=RGS_STATS_COLUMNS).astype(RGS_STATS_BASE_DTYPES)
433435
if not preserve_metadata:
434436
self._key_value_metadata = {}
437+
if release_lock:
438+
self._release_lock()
435439

436440
def to_pandas(self) -> DataFrame:
437441
"""
@@ -524,6 +528,15 @@ def _align_file_ids(self):
524528
# Set new ids.
525529
self._row_group_stats.loc[mask_ids_to_rename, KEY_FILE_IDS] = new_ids
526530

531+
def _release_lock(self):
532+
"""
533+
Release lock with reference counting.
534+
"""
535+
if self._lock._ref_count > 0:
536+
self._lock._ref_count -= 1
537+
if self._lock._ref_count == 0:
538+
self._lock.unlock(unconditionally=True)
539+
527540
def _remove_row_group_files(
528541
self,
529542
file_ids: List[int],

oups/store/ordered_parquet_dataset/ordered_parquet_dataset/read_only.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,12 @@ def __setattr__(self, name: str, value: Any):
174174
"""
175175
raise PermissionError(f"cannot set attribute '{name}' on a read-only dataset.")
176176

177+
def remove_from_disk(self, preserve_metadata=False):
178+
"""
179+
Block dataset removal.
180+
"""
181+
raise PermissionError("cannot call 'remove_from_disk' on a read-only dataset.")
182+
177183
def write(self, **kwargs):
178184
"""
179185
Block write operations.

oups/store/store/store.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,11 @@ def __delitem__(self, key: dataclass):
239239
Key specifying the location where to delete the data. It has to be
240240
an instance of the dataclass provided at Store instantiation.
241241
242+
Raises
243+
------
244+
KeyError
245+
If the key is not found in the store.
246+
242247
"""
243248
if key in self.keys:
244249
# Get OPD instance and remove its files
@@ -250,6 +255,8 @@ def __delitem__(self, key: dataclass):
250255
while (upper_dir != self.basepath) and (not list(upper_dir.iterdir())):
251256
upper_dir.rmdir()
252257
upper_dir = upper_dir.parent
258+
else:
259+
raise KeyError(f"key '{key}' not found in store.")
253260

254261
def __getitem__(self, key: dataclass) -> OrderedParquetDataset:
255262
"""
@@ -286,7 +293,8 @@ def get(self, key: dataclass, **kwargs) -> OrderedParquetDataset:
286293
Returns
287294
-------
288295
OrderedParquetDataset
289-
The OrderedParquetDataset instance corresponding to key.
296+
The OrderedParquetDataset instance corresponding to key. Creates a
297+
new OrderedParquetDataset object if the key doesn't exist.
290298
291299
"""
292300
opd = OrderedParquetDataset(self.basepath / key.to_path, **kwargs)

tests/test_store/test_ordered_parquet_dataset/test_opd_read_only.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,18 @@ def test_exception_readonly_permission_errors(tmp_path):
149149
):
150150
readonly_opd.ordered_on = "different_column"
151151

152-
# Test write() - should prevent writing data
152+
# Test remove_from_disk() - should prevent clearing dataset from disk.
153+
with pytest.raises(
154+
PermissionError,
155+
match="cannot call 'remove_from_disk' on a read-only dataset",
156+
):
157+
readonly_opd.remove_from_disk()
158+
159+
# Test write() - should prevent writing data.
153160
with pytest.raises(PermissionError, match="cannot call 'write' on a read-only dataset"):
154161
readonly_opd.write(df=df_ref)
155162

156-
# Test _align_file_ids() - should prevent file ID alignment
163+
# Test _align_file_ids() - should prevent file ID alignment.
157164
with pytest.raises(
158165
PermissionError,
159166
match="cannot call '_align_file_ids' on a read-only dataset",
@@ -167,14 +174,14 @@ def test_exception_readonly_permission_errors(tmp_path):
167174
):
168175
readonly_opd._remove_row_group_files(file_ids=[0])
169176

170-
# Test _sort_row_groups() - should prevent sorting
177+
# Test _sort_row_groups() - should prevent sorting.
171178
with pytest.raises(
172179
PermissionError,
173180
match="cannot call '_sort_row_groups' on a read-only dataset",
174181
):
175182
readonly_opd._sort_row_groups()
176183

177-
# Test _write_metadata_file() - should prevent metadata writing
184+
# Test _write_metadata_file() - should prevent metadata writing.
178185
with pytest.raises(
179186
PermissionError,
180187
match="cannot call '_write_metadata_file' on a read-only dataset",
@@ -188,7 +195,7 @@ def test_exception_readonly_permission_errors(tmp_path):
188195
):
189196
readonly_opd._write_row_group_files([df_ref])
190197

191-
# Verify that read operations still work
198+
# Verify that read operations still work.
192199
df_result = readonly_opd.to_pandas()
193200
assert len(df_result) == 2 # Should have 2 rows from the 2-row groups subset (indices 1 and 2)
194201
assert readonly_opd.ordered_on == "timestamp"

tests/test_store/test_store/test_store.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,3 +250,33 @@ class WeatherEntry:
250250
assert res[0][we2].equals(df2.iloc[:2].reset_index(drop=True))
251251
assert res[1][we2].equals(df2.iloc[2:2].reset_index(drop=True))
252252
assert res[2][we2].equals(df2.iloc[2:].reset_index(drop=True))
253+
254+
255+
def test_exception_store_delitem(tmp_path):
256+
"""
257+
Test that Store raises KeyError for non-existent keys in get, __getitem__, and
258+
__delitem__.
259+
"""
260+
261+
@toplevel
262+
class Indexer:
263+
category: str
264+
item: str
265+
266+
store = Store(tmp_path, Indexer)
267+
# Create a valid key and add some data
268+
valid_key = Indexer(category="data", item="dataset1")
269+
df = DataFrame({"timestamp": [1, 2, 3], "value": [10, 20, 30]})
270+
store[valid_key].write(df=df, ordered_on="timestamp")
271+
# Create a new key not in store.
272+
invalid_key = Indexer(category="missing", item="notfound")
273+
# Test that __delitem__ raises KeyError.
274+
with pytest.raises(KeyError, match="not found"):
275+
del store[invalid_key]
276+
# Verify valid key still works.
277+
assert valid_key in store
278+
assert len(store[valid_key].row_group_stats) > 0
279+
# Test after deletion, accessing should raise KeyError.
280+
del store[valid_key]
281+
with pytest.raises(KeyError, match="not found"):
282+
del store[valid_key]

0 commit comments

Comments
 (0)