Skip to content

Commit 3d239f1

Browse files
refactor: remove dry_run from delete() and drop()
The safemode prompt already provides a safer preview-and-confirm workflow: it executes within a transaction, shows all affected tables and row counts, and rolls back if the user declines. dry_run was a weaker alternative (pre-transaction count that could be stale) that added a union return type and dead parameters. For programmatic preview, use Diagram.cascade().counts() directly. The test_delete_dry_run, test_drop_dry_run, and test_part_delete_dry_run tests are replaced with test_delete_preview_with_counts which tests the Diagram.cascade().counts() API. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 070e029 commit 3d239f1

File tree

3 files changed

+24
-110
lines changed

3 files changed

+24
-110
lines changed

src/datajoint/table.py

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -981,15 +981,24 @@ def delete(
981981
transaction: bool = True,
982982
prompt: bool | None = None,
983983
part_integrity: str = "enforce",
984-
dry_run: bool = False,
985-
) -> int | dict[str, int]:
984+
) -> int:
986985
"""
987986
Deletes the contents of the table and its dependent tables, recursively.
988987
989988
Uses graph-driven cascade: builds a dependency diagram, propagates
990989
restrictions to all descendants, then deletes in reverse topological
991990
order (leaves first).
992991
992+
With ``safemode=True`` (the default), delete previews all affected
993+
tables and row counts, executes within a transaction, and asks for
994+
confirmation before committing. Declining rolls back all changes —
995+
effectively a built-in dry run.
996+
997+
To preview cascade impact without executing, use ``Diagram``::
998+
999+
diag = dj.Diagram(schema)
1000+
diag.cascade(MyTable & restriction).counts()
1001+
9931002
Args:
9941003
transaction: If `True`, use of the entire delete becomes an atomic transaction.
9951004
This is the default and recommended behavior. Set to `False` if this delete is
@@ -1000,12 +1009,9 @@ def delete(
10001009
- ``"enforce"`` (default): Error if parts would be deleted without masters.
10011010
- ``"ignore"``: Allow deleting parts without masters (breaks integrity).
10021011
- ``"cascade"``: Also delete masters when parts are deleted (maintains integrity).
1003-
dry_run: If `True`, return a dict mapping full table names to affected
1004-
row counts without deleting any data. Default False.
10051012
10061013
Returns:
1007-
Number of deleted rows (excluding those from dependent tables), or
1008-
(if ``dry_run``) a dict mapping full table name to affected row count.
1014+
Number of deleted rows (excluding those from dependent tables).
10091015
10101016
Raises:
10111017
DataJointError: When deleting within an existing transaction.
@@ -1019,9 +1025,6 @@ def delete(
10191025
diagram = Diagram._from_table(self)
10201026
diagram = diagram.cascade(self, part_integrity=part_integrity)
10211027

1022-
if dry_run:
1023-
return diagram.counts()
1024-
10251028
conn = self.connection
10261029
prompt = conn._config["safemode"] if prompt is None else prompt
10271030

@@ -1145,25 +1148,22 @@ def drop_quick(self):
11451148
else:
11461149
logger.info("Nothing to drop: table %s is not declared" % self.full_table_name)
11471150

1148-
def drop(self, prompt: bool | None = None, part_integrity: str = "enforce", dry_run: bool = False):
1151+
def drop(self, prompt: bool | None = None, part_integrity: str = "enforce"):
11491152
"""
11501153
Drop the table and all tables that reference it, recursively.
11511154
11521155
Uses graph-driven traversal: builds a dependency diagram and drops
11531156
in reverse topological order (leaves first).
11541157
1158+
With ``safemode=True`` (the default), drop previews all affected
1159+
tables and row counts and asks for confirmation before proceeding.
1160+
11551161
Args:
11561162
prompt: If `True`, show what will be dropped and ask for confirmation.
11571163
If `False`, drop without confirmation. Default is `dj.config['safemode']`.
11581164
part_integrity: Policy for master-part integrity. One of:
11591165
- ``"enforce"`` (default): Error if parts would be dropped without masters.
11601166
- ``"ignore"``: Allow dropping parts without masters.
1161-
dry_run: If `True`, return a dict mapping full table names to row
1162-
counts without dropping any tables. Default False.
1163-
1164-
Returns:
1165-
dict[str, int] or None: If ``dry_run``, mapping of full table name
1166-
to row count. Otherwise None.
11671167
"""
11681168
if self.restriction:
11691169
raise DataJointError(
@@ -1187,14 +1187,6 @@ def drop(self, prompt: bool | None = None, part_integrity: str = "enforce", dry_
11871187
)
11881188
)
11891189

1190-
if dry_run:
1191-
result = {}
1192-
for ft in diagram:
1193-
count = len(ft)
1194-
result[ft.full_table_name] = count
1195-
logger.info("{table} ({count} tuples)".format(table=ft.full_table_name, count=count))
1196-
return result
1197-
11981190
do_drop = True
11991191
if prompt:
12001192
for ft in diagram:

src/datajoint/user_tables.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ def delete(self, part_integrity: str = "enforce", **kwargs):
224224
- ``"ignore"``: Allow direct deletion (breaks master-part integrity).
225225
- ``"cascade"``: Delete parts AND cascade up to delete master.
226226
**kwargs: Additional arguments passed to Table.delete()
227-
(transaction, prompt, dry_run)
227+
(transaction, prompt)
228228
229229
Raises:
230230
DataJointError: If part_integrity="enforce" (direct Part deletes prohibited)
@@ -237,7 +237,7 @@ def delete(self, part_integrity: str = "enforce", **kwargs):
237237
)
238238
return super().delete(part_integrity=part_integrity, **kwargs)
239239

240-
def drop(self, part_integrity: str = "enforce", dry_run: bool = False):
240+
def drop(self, part_integrity: str = "enforce"):
241241
"""
242242
Drop a Part table.
243243
@@ -246,13 +246,12 @@ def drop(self, part_integrity: str = "enforce", dry_run: bool = False):
246246
- ``"enforce"`` (default): Error - drop master instead.
247247
- ``"ignore"``: Allow direct drop (breaks master-part structure).
248248
Note: ``"cascade"`` is not supported for drop (too destructive).
249-
dry_run: If `True`, return row counts without dropping. Default False.
250249
251250
Raises:
252251
DataJointError: If part_integrity="enforce" (direct Part drops prohibited)
253252
"""
254253
if part_integrity == "ignore":
255-
return super().drop(part_integrity="ignore", dry_run=dry_run)
254+
return super().drop(part_integrity="ignore")
256255
elif part_integrity == "enforce":
257256
raise DataJointError("Cannot drop a Part directly. Drop master instead, or use part_integrity='ignore' to force.")
258257
else:

tests/integration/test_cascade_delete.py

Lines changed: 5 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,8 @@ class Observation(dj.Manual):
190190
assert remaining_obs[0]["measurement"] == 15.3
191191

192192

193-
def test_delete_dry_run(schema_by_backend):
194-
"""dry_run=True returns affected row counts without deleting data."""
193+
def test_delete_preview_with_counts(schema_by_backend):
194+
"""Diagram.cascade().counts() previews affected rows without deleting."""
195195

196196
@schema_by_backend
197197
class Parent(dj.Manual):
@@ -216,8 +216,9 @@ class Child(dj.Manual):
216216
Child.insert1((1, 2, "C1-2"))
217217
Child.insert1((2, 1, "C2-1"))
218218

219-
# dry_run on restricted delete
220-
counts = (Parent & {"parent_id": 1}).delete(dry_run=True)
219+
# Preview restricted cascade via Diagram
220+
diag = dj.Diagram._from_table(Parent & {"parent_id": 1})
221+
counts = diag.cascade(Parent & {"parent_id": 1}).counts()
221222

222223
assert isinstance(counts, dict)
223224
assert counts[Parent.full_table_name] == 1
@@ -226,81 +227,3 @@ class Child(dj.Manual):
226227
# Data must still be intact
227228
assert len(Parent()) == 2
228229
assert len(Child()) == 3
229-
230-
# dry_run on unrestricted delete
231-
counts_all = Parent.delete(dry_run=True)
232-
assert counts_all[Parent.full_table_name] == 2
233-
assert counts_all[Child.full_table_name] == 3
234-
235-
# Still intact
236-
assert len(Parent()) == 2
237-
assert len(Child()) == 3
238-
239-
240-
def test_drop_dry_run(schema_by_backend):
241-
"""dry_run=True returns row counts without dropping tables."""
242-
243-
@schema_by_backend
244-
class Parent(dj.Manual):
245-
definition = """
246-
parent_id : int
247-
---
248-
name : varchar(255)
249-
"""
250-
251-
@schema_by_backend
252-
class Child(dj.Manual):
253-
definition = """
254-
-> Parent
255-
child_id : int
256-
---
257-
data : varchar(255)
258-
"""
259-
260-
Parent.insert1((1, "P1"))
261-
Child.insert1((1, 1, "C1"))
262-
263-
counts = Parent.drop(dry_run=True)
264-
265-
assert isinstance(counts, dict)
266-
assert counts[Parent.full_table_name] == 1
267-
assert counts[Child.full_table_name] == 1
268-
269-
# Tables must still exist and have data
270-
assert Parent.is_declared
271-
assert Child.is_declared
272-
assert len(Parent()) == 1
273-
assert len(Child()) == 1
274-
275-
276-
def test_part_delete_dry_run(schema_by_backend):
277-
"""dry_run=True on Part.delete() returns affected row counts without deleting."""
278-
279-
@schema_by_backend
280-
class Master(dj.Manual):
281-
definition = """
282-
master_id : int
283-
---
284-
name : varchar(255)
285-
"""
286-
287-
class Detail(dj.Part):
288-
definition = """
289-
-> master
290-
detail_id : int
291-
---
292-
data : varchar(255)
293-
"""
294-
295-
Master.insert1((1, "M1"))
296-
Master.Detail.insert([(1, 1, "D1"), (1, 2, "D2")])
297-
298-
# dry_run with part_integrity="ignore" should return counts without deleting
299-
counts = (Master.Detail & {"master_id": 1}).delete(part_integrity="ignore", dry_run=True)
300-
301-
assert isinstance(counts, dict)
302-
assert counts[Master.Detail.full_table_name] == 2
303-
304-
# Data must still be intact
305-
assert len(Master()) == 1
306-
assert len(Master.Detail()) == 2

0 commit comments

Comments
 (0)