Store.delete has inconsistent implementations

LocalStore.delete removes entire directory trees, i.e. many objects:

Lines 206 to 213 in 4c3081c

    
           async def delete(self, key: str) -> None: 
        
               # docstring inherited 
        
               self._check_writable() 
        
               path = self.root / key 
        
               if path.is_dir():  # TODO: support deleting directories? shutil.rmtree? 
        
                   shutil.rmtree(path) 
        
               else: 
        
                   await asyncio.to_thread(path.unlink, True)  # Q: we may want to raise if path is missing

MemoryStore.delete removes single keys, i.e. just one object:

zarr-python/src/zarr/storage/memory.py

Lines 134 to 140 in 4c3081c

    
           async def delete(self, key: str) -> None: 
        
               # docstring inherited 
        
               self._check_writable() 
        
               try: 
        
                   del self._store_dict[key] 
        
               except KeyError: 
        
                   pass

We should pick one of the two options. The fact that we got this far with such skew also means that our store tests should be made more extensive.

Good catch! Store.delete should be for a single key. In #2430, I introduce delete_dir for removing all objects below a node in a hierarchy.

Navigation Menu

Repository navigation

`Store.delete` has inconsistent implementations #2451

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	async def delete(self, key: str) -> None:
	# docstring inherited
	self._check_writable()
	path = self.root / key
	if path.is_dir(): # TODO: support deleting directories? shutil.rmtree?
	shutil.rmtree(path)
	else:
	await asyncio.to_thread(path.unlink, True) # Q: we may want to raise if path is missing

Store.delete has inconsistent implementations #2451

Description

Activity

jhamman commented on Oct 30, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

`Store.delete` has inconsistent implementations #2451