samredai commented on a change in pull request #3450:
URL: https://github.com/apache/iceberg/pull/3450#discussion_r817965361
##########
File path: python/src/iceberg/types.py
##########
@@ -43,6 +46,11 @@ def __new__(cls, *args, **kwargs):
return cls._instance
+class Truncatable:
+ def truncate(self, value, width: int):
+ raise ValueError(f"Cannot truncate type: {self}")
Review comment:
Ok I see now the idea is to use the class for type annotations. In that
case I'm just wondering how we would ever hit this `ValueError` if there are no
mypy typecheck failures then plus the fact that everything that inherits from
`Truncatable` will indeed override the truncate method.
I think the goal is to communicate that this is an abc class and an abstract
method, maybe we can do that more formally?
```py
from abc import ABC, abstractmethod
class Truncatable(ABC):
@abstractmethod
def truncate(self, value, width: int):
...
```
The added benefit here is that if for some reason a class does not override
the `truncate` method, that will fail immediately during init:
```py
class Foo(Truncatable):
pass
Foo() # TypeError: Can't instantiate abstract class Foo with abstract
method truncate
```
instead of waiting until the `truncate` method is accessed later on to raise
the `ValueError`.
--
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]
