dramaticlly commented on a change in pull request #4262: URL: https://github.com/apache/iceberg/pull/4262#discussion_r829721121
########## File path: python/src/iceberg/expression/literals.py ########## @@ -0,0 +1,654 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import datetime +import sys +import uuid +from decimal import ROUND_HALF_UP, Decimal +from functools import singledispatch +from typing import Any + +from iceberg.types import PrimitiveType + +if sys.version_info >= (3, 8): + from functools import singledispatchmethod # pragma: no cover +else: + from singledispatch import singledispatchmethod # pragma: no cover + +import pytz + +from iceberg.types import ( + BinaryType, + BooleanType, + DateType, + DecimalType, + DoubleType, + FixedType, + FloatType, + IntegerType, + LongType, + Singleton, + StringType, + TimestampType, + TimestamptzType, + TimeType, + UUIDType, +) + +EPOCH = datetime.datetime.utcfromtimestamp(0) + +""" +Iceberg literal is wrapper class used in expressions, which return unbound predicates +It's being organized as below +Literal +|-- AboveMax +|-- BelowMin +|-- BaseLiteral + |-- StringLiteral + |-- FixedLiteral + |-- BinaryLiteral + |-- ComparableLiteral + |-- BooleanLiteral + |-- IntegerLiteral + |-- LongLiteral + |-- FloatLiteral + |-- DoubleLiteral + |-- DateLiteral + |-- TimeLiteral + |-- TimestampLiteral + |-- DecimalLiteral + |-- UUIDLiteral +""" + + +class Literal: + def to(self, type_var: PrimitiveType): + raise NotImplementedError() + + +class BaseLiteral(Literal): + """Base literal which has a value and can be converted between types""" + + def __init__(self, repr_string: str, value): + self._repr_string = repr_string + if value is None: + raise TypeError("Cannot set value of BaseLiteral to None") + self._value = value + + def to(self, type_var): + raise NotImplementedError() + + @property + def value(self): + return self._value + + def __repr__(self): + return self._repr_string + + def __str__(self): + return str(self._value) + + +@singledispatch +def literal(value: Any) -> BaseLiteral: + """ + A generic Literal factory to construct an iceberg Literal based on python primitive data type + using dynamic overloading + + Args: + value(python primitive type): the value to be associated with literal + + Example: + from iceberg.expressions.literals import literal + >>> literal(123) + <IntegerLiteral(123)>v + """ + raise TypeError(f"Unimplemented Type Literal for value: {value}") + + [email protected] +def _(value: bool): + return BooleanLiteral(value) + + [email protected] +def _(value: int): + """ + Upgrade to long if python int is outside the JAVA_MIN_INT and JAVA_MAX_INT + """ + if value < IntegerType.min or value > IntegerType.max: + return LongLiteral(value) + return IntegerLiteral(value) + + [email protected] +def _(value: float): + """ + Upgrade to double if python float is outside the JAVA_MIN_FLOAT and JAVA_MAX_FLOAT + """ + if value < FloatType.min or value > FloatType.max: + return DoubleLiteral(value) + return FloatLiteral(value) + + [email protected] +def _(value: str): + return StringLiteral(value) + + [email protected] +def _(value: uuid.UUID): + return UUIDLiteral(value) + + [email protected] +def _(value: bytes): + return FixedLiteral(value) + + [email protected] +def _(value: bytearray): + return BinaryLiteral(value) + + [email protected] +def _(value: Decimal): + return DecimalLiteral(value) + + +class ComparableLiteral(BaseLiteral): + def __init__(self, repr_string: str, value): + super(ComparableLiteral, self).__init__(repr_string, value) + + def to(self, type_var): + raise NotImplementedError() + + def __eq__(self, other): + return self.value == other.value + + def __ne__(self, other): + return not self.__eq__(other) + + def __lt__(self, other): + return self.value < other.value + + def __gt__(self, other): + return self.value > other.value + + def __le__(self, other): + return self.value <= other.value + + def __ge__(self, other): + return self.value >= other.value + + +class AboveMax(Literal, Singleton): + def value(self): + raise RuntimeError("AboveMax has no value") + + def to(self, type_var): + raise RuntimeError("Cannot change the type of AboveMax") + + def __str__(self): + return "aboveMax" + + +class BelowMin(Literal, Singleton): + def value(self): + raise RuntimeError("BelowMin has no value") + + def to(self, type_var): + raise RuntimeError("Cannot change the type of BelowMin") + + def __str__(self): + return "belowMin" + + +class BooleanLiteral(ComparableLiteral): + def __init__(self, value): + super(BooleanLiteral, self).__init__(f"BooleanLiteral({value})", value) + + @singledispatchmethod + def to(self, type_var): + return None + + @to.register(BooleanType) + def _(self, type_var): + return self + + +class IntegerLiteral(ComparableLiteral): + def __init__(self, value): + super(IntegerLiteral, self).__init__(f"IntegerLiteral({value})", value) + + @singledispatchmethod + def to(self, type_var): + return None + + @to.register(IntegerType) + def _(self, type_var): + return self + + @to.register(LongType) + def _(self, type_var): + return LongLiteral(self.value) + + @to.register(FloatType) + def _(self, type_var): + return FloatLiteral(self.value) + + @to.register(DoubleType) + def _(self, type_var): + return DoubleLiteral(self.value) + + @to.register(DateType) + def _(self, type_var): + return DateLiteral(self.value) + + @to.register(DecimalType) + def _(self, type_var): + if type_var.scale == 0: + return DecimalLiteral(Decimal(self.value)) + else: + return DecimalLiteral( + Decimal(self.value).quantize( + Decimal("." + "".join(["0" for _ in range(1, type_var.scale)]) + "1"), rounding=ROUND_HALF_UP Review comment: yeah I was looking into [Decimal](https://docs.python.org/3/library/decimal.html#) and looks like the quantize is the API supposed to round the Decimal value to its first operant. I can kind of see that construct the Decimal from String is for better precision compare to float counterpart. I am not very familiar with the Decimal and did not find a better API to do this, I guess we can keep this as of now and come back if someone find a better way to do this ? What do you think ########## File path: python/tox.ini ########## @@ -65,6 +69,9 @@ commands = [testenv:type-check] deps = mypy + types-pytz Review comment: Thank you for sharing the mypy usage, I removed those 2 and change the commands below to ` mypy --no-implicit-optional --install-types --non-interactive --config tox.ini src` ########## File path: python/tox.ini ########## @@ -25,6 +25,10 @@ deps = mock pytest pyarrow + pytz Review comment: Thank you @Fokko , looks like for dependency already exists in `setup.cfg`, I did not need to repeat it again in testenv here, removed them as suggested ########## File path: python/src/iceberg/expression/literals.py ########## @@ -0,0 +1,654 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import datetime +import sys +import uuid +from decimal import ROUND_HALF_UP, Decimal +from functools import singledispatch +from typing import Any + +from iceberg.types import PrimitiveType + +if sys.version_info >= (3, 8): + from functools import singledispatchmethod # pragma: no cover +else: + from singledispatch import singledispatchmethod # pragma: no cover + +import pytz + +from iceberg.types import ( + BinaryType, + BooleanType, + DateType, + DecimalType, + DoubleType, + FixedType, + FloatType, + IntegerType, + LongType, + Singleton, + StringType, + TimestampType, + TimestamptzType, + TimeType, + UUIDType, +) + +EPOCH = datetime.datetime.utcfromtimestamp(0) + +""" +Iceberg literal is wrapper class used in expressions, which return unbound predicates +It's being organized as below +Literal +|-- AboveMax +|-- BelowMin +|-- BaseLiteral + |-- StringLiteral + |-- FixedLiteral + |-- BinaryLiteral + |-- ComparableLiteral Review comment: Yeah I think that's a good point, let me change the parent of StringLiteral ########## File path: python/src/iceberg/expression/literals.py ########## @@ -0,0 +1,654 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import datetime +import sys +import uuid +from decimal import ROUND_HALF_UP, Decimal +from functools import singledispatch +from typing import Any + +from iceberg.types import PrimitiveType + +if sys.version_info >= (3, 8): + from functools import singledispatchmethod # pragma: no cover +else: + from singledispatch import singledispatchmethod # pragma: no cover + +import pytz + +from iceberg.types import ( + BinaryType, + BooleanType, + DateType, + DecimalType, + DoubleType, + FixedType, + FloatType, + IntegerType, + LongType, + Singleton, + StringType, + TimestampType, + TimestamptzType, + TimeType, + UUIDType, +) + +EPOCH = datetime.datetime.utcfromtimestamp(0) + +""" +Iceberg literal is wrapper class used in expressions, which return unbound predicates +It's being organized as below +Literal +|-- AboveMax +|-- BelowMin +|-- BaseLiteral + |-- StringLiteral + |-- FixedLiteral + |-- BinaryLiteral + |-- ComparableLiteral + |-- BooleanLiteral + |-- IntegerLiteral + |-- LongLiteral + |-- FloatLiteral + |-- DoubleLiteral + |-- DateLiteral + |-- TimeLiteral + |-- TimestampLiteral + |-- DecimalLiteral + |-- UUIDLiteral +""" + + +class Literal: + def to(self, type_var: PrimitiveType): + raise NotImplementedError() + + +class BaseLiteral(Literal): + """Base literal which has a value and can be converted between types""" + + def __init__(self, repr_string: str, value): + self._repr_string = repr_string + if value is None: + raise TypeError("Cannot set value of BaseLiteral to None") + self._value = value + + def to(self, type_var): + raise NotImplementedError() + + @property + def value(self): + return self._value + + def __repr__(self): + return self._repr_string + + def __str__(self): + return str(self._value) + + +@singledispatch +def literal(value: Any) -> BaseLiteral: + """ + A generic Literal factory to construct an iceberg Literal based on python primitive data type + using dynamic overloading + + Args: + value(python primitive type): the value to be associated with literal + + Example: + from iceberg.expressions.literals import literal + >>> literal(123) + <IntegerLiteral(123)>v + """ + raise TypeError(f"Unimplemented Type Literal for value: {value}") + + [email protected] +def _(value: bool): + return BooleanLiteral(value) + + [email protected] +def _(value: int): + """ + Upgrade to long if python int is outside the JAVA_MIN_INT and JAVA_MAX_INT + """ + if value < IntegerType.min or value > IntegerType.max: + return LongLiteral(value) + return IntegerLiteral(value) + + [email protected] +def _(value: float): + """ + Upgrade to double if python float is outside the JAVA_MIN_FLOAT and JAVA_MAX_FLOAT + """ + if value < FloatType.min or value > FloatType.max: + return DoubleLiteral(value) + return FloatLiteral(value) + + [email protected] +def _(value: str): + return StringLiteral(value) + + [email protected] +def _(value: uuid.UUID): + return UUIDLiteral(value) + + [email protected] +def _(value: bytes): + return FixedLiteral(value) + + [email protected] +def _(value: bytearray): + return BinaryLiteral(value) + + [email protected] +def _(value: Decimal): + return DecimalLiteral(value) + + +class ComparableLiteral(BaseLiteral): + def __init__(self, repr_string: str, value): + super(ComparableLiteral, self).__init__(repr_string, value) + + def to(self, type_var): + raise NotImplementedError() + + def __eq__(self, other): + return self.value == other.value + + def __ne__(self, other): + return not self.__eq__(other) + + def __lt__(self, other): + return self.value < other.value + + def __gt__(self, other): + return self.value > other.value + + def __le__(self, other): + return self.value <= other.value + + def __ge__(self, other): + return self.value >= other.value + + +class AboveMax(Literal, Singleton): + def value(self): + raise RuntimeError("AboveMax has no value") + + def to(self, type_var): + raise RuntimeError("Cannot change the type of AboveMax") + + def __str__(self): + return "aboveMax" Review comment: I directly port over the old code, but I also dont see why it have to be in lower case, Capitalize as suggested -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
