This is an automated email from the ASF dual-hosted git repository.
dweeks pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/master by this push:
new a38e66323a Python: Don't use a metaclass for the Singleton (#5055)
a38e66323a is described below
commit a38e66323a99b65f433cb6e96007674a741a2f5a
Author: Fokko Driesprong <[email protected]>
AuthorDate: Wed Jun 15 23:07:43 2022 +0200
Python: Don't use a metaclass for the Singleton (#5055)
Metaclasses in Python don't mix very well, and when they cause
conflicts then it breaks at initialization.
Example of metaclasses is the ABC, but also the Pydantic BaseModel.
I tried to circumvent this by extending the Singleton from ABC
but it is likely that we use different metaclasses in the future,
and then it will bring us sadness and misery.
---
python/src/iceberg/expressions/base.py | 12 ++++++------
python/src/iceberg/expressions/literals.py | 4 ++--
python/src/iceberg/types.py | 3 ++-
python/src/iceberg/utils/singleton.py | 8 ++++----
python/tests/expressions/test_expressions_base.py | 4 ++--
5 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/python/src/iceberg/expressions/base.py
b/python/src/iceberg/expressions/base.py
index 06cc4d2bdb..919cc09b86 100644
--- a/python/src/iceberg/expressions/base.py
+++ b/python/src/iceberg/expressions/base.py
@@ -14,7 +14,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from abc import ABCMeta, abstractmethod
+from abc import ABC, abstractmethod
from enum import Enum, auto
from functools import reduce, singledispatch
from typing import Any, Generic, TypeVar
@@ -89,7 +89,7 @@ OPERATION_NEGATIONS = {
}
-class Literal(Generic[T], metaclass=ABCMeta):
+class Literal(Generic[T], ABC):
"""Literal which has a value and can be converted between types"""
def __init__(self, value: T, value_type: type):
@@ -130,7 +130,7 @@ class Literal(Generic[T], metaclass=ABCMeta):
return self.value >= other.value
-class BooleanExpression(metaclass=ABCMeta):
+class BooleanExpression(ABC):
"""base class for all boolean expressions"""
@abstractmethod
@@ -242,7 +242,7 @@ class Not(BooleanExpression):
return f"(not {self.child})"
-class AlwaysTrue(BooleanExpression, metaclass=Singleton):
+class AlwaysTrue(BooleanExpression, ABC, Singleton):
"""TRUE expression"""
def __invert__(self) -> "AlwaysFalse":
@@ -255,7 +255,7 @@ class AlwaysTrue(BooleanExpression, metaclass=Singleton):
return "true"
-class AlwaysFalse(BooleanExpression, metaclass=Singleton):
+class AlwaysFalse(BooleanExpression, ABC, Singleton):
"""FALSE expression"""
def __invert__(self) -> "AlwaysTrue":
@@ -349,7 +349,7 @@ class UnboundReference:
return BoundReference(field=field,
accessor=schema.accessor_for_field(field.field_id))
-class BooleanExpressionVisitor(Generic[T], metaclass=ABCMeta):
+class BooleanExpressionVisitor(Generic[T], ABC):
@abstractmethod
def visit_true(self) -> T:
"""Visit method for an AlwaysTrue boolean expression
diff --git a/python/src/iceberg/expressions/literals.py
b/python/src/iceberg/expressions/literals.py
index c89894d8ad..2f0af3b6e0 100644
--- a/python/src/iceberg/expressions/literals.py
+++ b/python/src/iceberg/expressions/literals.py
@@ -112,7 +112,7 @@ def _(value: Decimal) -> Literal[Decimal]:
return DecimalLiteral(value)
-class AboveMax(metaclass=Singleton):
+class AboveMax(Singleton):
@property
def value(self):
raise ValueError("AboveMax has no value")
@@ -127,7 +127,7 @@ class AboveMax(metaclass=Singleton):
return "AboveMax"
-class BelowMin(metaclass=Singleton):
+class BelowMin(Singleton):
def __init__(self):
pass
diff --git a/python/src/iceberg/types.py b/python/src/iceberg/types.py
index 3139d13f85..d665dbcae6 100644
--- a/python/src/iceberg/types.py
+++ b/python/src/iceberg/types.py
@@ -29,6 +29,7 @@ Example:
Notes:
- https://iceberg.apache.org/#spec/#primitive-types
"""
+from abc import ABC
from dataclasses import dataclass, field
from functools import cached_property
from typing import ClassVar, Optional, Tuple
@@ -37,7 +38,7 @@ from iceberg.utils.singleton import Singleton
@dataclass(frozen=True)
-class IcebergType(metaclass=Singleton):
+class IcebergType(ABC, Singleton):
"""Base type for all Iceberg Types
Example:
diff --git a/python/src/iceberg/utils/singleton.py
b/python/src/iceberg/utils/singleton.py
index f6c6912fab..c36155de14 100644
--- a/python/src/iceberg/utils/singleton.py
+++ b/python/src/iceberg/utils/singleton.py
@@ -14,15 +14,15 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from abc import ABCMeta
+
from typing import ClassVar, Dict
-class Singleton(ABCMeta):
+class Singleton:
_instances: ClassVar[Dict] = {}
- def __call__(cls, *args, **kwargs):
+ def __new__(cls, *args, **kwargs):
key = (cls, args, tuple(sorted(kwargs.items())))
if key not in cls._instances:
- cls._instances[key] = super().__call__(*args, **kwargs)
+ cls._instances[key] = super().__new__(cls)
return cls._instances[key]
diff --git a/python/tests/expressions/test_expressions_base.py
b/python/tests/expressions/test_expressions_base.py
index 3d9177bbfe..773d7ecc53 100644
--- a/python/tests/expressions/test_expressions_base.py
+++ b/python/tests/expressions/test_expressions_base.py
@@ -64,7 +64,7 @@ def test_raise_on_no_negation_for_operation(operation):
assert str(exc_info.value) == f"No negation defined for operation
{operation}"
-class TestExpressionA(base.BooleanExpression, metaclass=Singleton):
+class TestExpressionA(base.BooleanExpression, Singleton):
def __invert__(self):
return TestExpressionB()
@@ -75,7 +75,7 @@ class TestExpressionA(base.BooleanExpression,
metaclass=Singleton):
return "testexpra"
-class TestExpressionB(base.BooleanExpression, metaclass=Singleton):
+class TestExpressionB(base.BooleanExpression, Singleton):
def __invert__(self):
return TestExpressionA()