Fokko commented on code in PR #6139:
URL: https://github.com/apache/iceberg/pull/6139#discussion_r1022019082
##########
python/pyiceberg/expressions/__init__.py:
##########
@@ -48,12 +64,13 @@ class Bound(ABC):
"""Represents a bound value expression"""
-class Unbound(Generic[B], ABC):
+class Unbound(ABC):
"""Represents an unbound value expression"""
+ @property
@abstractmethod
- def bind(self, schema: Schema, case_sensitive: bool = True) -> B:
- ... # pragma: no cover
+ def as_bound(self) -> Type[Bound]:
Review Comment:
I think so. Because we can use OOP now (because we've removed the data
classes), we're able to solve this using inheritance in a cleaner way. Also, we
remove all the types and return always a `BooleanExpression` after binding,
which makes us lose the type information.
Also, we're not able to pass in the `[T]`:
```python
@dataclass(frozen=True)
class GreaterThan(LiteralPredicate[T]):
as_bound = BoundGreaterThan[T]
```
Produces:
```
python/pyiceberg/expressions/__init__.py:518: error: Can't use bound type
variable "T" to define generic alias
Found 1 error in 1 file (checked 83 source files)
```
But most importantly, we get no type checking at all. On current master, if
I add some bogus arguments:
```python
@dataclass(frozen=True)
class UnaryPredicate(UnboundPredicate[T]):
def bind(self, schema: Schema, case_sensitive: bool = True) ->
BooleanExpression:
bound_term = self.term.bind(schema, case_sensitive)
return self.as_bound(bound_term, 1, 2, 3) # The int arguments are
not expected
```
It just runs fine:
```
➜ python git:(master) ✗ pre-commit run --all-files mypy
mypy.....................................................................Passed
```
If I do the same on this branch:
```
➜ python git:(fd-remove-dataclass) pre-commit run --all-files mypy
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1
python/pyiceberg/expressions/__init__.py:325: error: Too many arguments for
"BoundUnaryPredicate"
Found 1 error in 1 file (checked 83 source files)
```
--
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]
