Skip to content

Commit e992078

Browse files
committed
Simplify deepcopy fix to per-class overrides on And, Or, Not
Instead of a generic __deepcopy__ on BooleanExpression with Pydantic internals, add simple __deepcopy__ methods to the three classes that need them. Also add identity assertions for non-Singleton copies.
1 parent 013fa11 commit e992078

2 files changed

Lines changed: 16 additions & 7 deletions

File tree

pyiceberg/expressions/__init__.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,6 @@ def _to_literal(value: L | Literal[L]) -> Literal[L]:
5252
class BooleanExpression(IcebergBaseModel, ABC):
5353
"""An expression that evaluates to a boolean."""
5454

55-
def __deepcopy__(self, memo: dict[int, Any]) -> BooleanExpression:
56-
"""Return a deep copy of this expression."""
57-
if isinstance(self, Singleton):
58-
return self
59-
fields = {name: copy.deepcopy(getattr(self, name), memo) for name in type(self).model_fields}
60-
return type(self)(**fields)
61-
6255
@abstractmethod
6356
def __invert__(self) -> BooleanExpression:
6457
"""Transform the Expression into its negated version."""
@@ -347,6 +340,10 @@ def __invert__(self) -> BooleanExpression:
347340
# De Morgan's law: not (A and B) = (not A) or (not B)
348341
return Or(~self.left, ~self.right)
349342

343+
def __deepcopy__(self, memo: dict[int, Any]) -> And:
344+
"""Return a deep copy of the And expression."""
345+
return And(copy.deepcopy(self.left, memo), copy.deepcopy(self.right, memo))
346+
350347
def __getnewargs__(self) -> tuple[BooleanExpression, BooleanExpression]:
351348
"""Pickle the And class."""
352349
return (self.left, self.right)
@@ -394,6 +391,10 @@ def __invert__(self) -> BooleanExpression:
394391
# De Morgan's law: not (A or B) = (not A) and (not B)
395392
return And(~self.left, ~self.right)
396393

394+
def __deepcopy__(self, memo: dict[int, Any]) -> Or:
395+
"""Return a deep copy of the Or expression."""
396+
return Or(copy.deepcopy(self.left, memo), copy.deepcopy(self.right, memo))
397+
397398
def __getnewargs__(self) -> tuple[BooleanExpression, BooleanExpression]:
398399
"""Pickle the Or class."""
399400
return (self.left, self.right)
@@ -436,6 +437,10 @@ def __invert__(self) -> BooleanExpression:
436437
"""Transform the Expression into its negated version."""
437438
return self.child
438439

440+
def __deepcopy__(self, memo: dict[int, Any]) -> Not:
441+
"""Return a deep copy of the Not expression."""
442+
return Not(copy.deepcopy(self.child, memo))
443+
439444
def __getnewargs__(self) -> tuple[BooleanExpression]:
440445
"""Pickle the Not class."""
441446
return (self.child,)

tests/expressions/test_expressions.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,24 +1300,28 @@ def test_deepcopy_and() -> None:
13001300
expr = And(EqualTo("x", 1), EqualTo("y", 2))
13011301
copied = copy.deepcopy(expr)
13021302
assert copied == expr
1303+
assert copied is not expr
13031304

13041305

13051306
def test_deepcopy_or() -> None:
13061307
expr = Or(EqualTo("x", 1), EqualTo("y", 2))
13071308
copied = copy.deepcopy(expr)
13081309
assert copied == expr
1310+
assert copied is not expr
13091311

13101312

13111313
def test_deepcopy_not() -> None:
13121314
expr = Not(EqualTo("x", 1))
13131315
copied = copy.deepcopy(expr)
13141316
assert copied == expr
1317+
assert copied is not expr
13151318

13161319

13171320
def test_deepcopy_equal_to() -> None:
13181321
expr = EqualTo("x", 1)
13191322
copied = copy.deepcopy(expr)
13201323
assert copied == expr
1324+
assert copied is not expr
13211325

13221326

13231327
def test_deepcopy_always_true() -> None:

0 commit comments

Comments
 (0)