smaheshwar-pltr commented on code in PR #3364:
URL: https://github.com/apache/iceberg-python/pull/3364#discussion_r3260788825
##########
pyiceberg/table/__init__.py:
##########
@@ -1685,20 +1750,19 @@ def _parse_row_filter(expr: str | BooleanExpression) ->
BooleanExpression:
return parser.parse(expr) if isinstance(expr, str) else expr
-S = TypeVar("S", bound="TableScan", covariant=True)
+A = TypeVar("A", bound="BaseScan", covariant=True)
+
+class BaseScan(ABC):
Review Comment:
Also pointing out: I could've avoided changing existing code entirely and
having a completely independent class for append scans with duplicated manifest
planning logic. I felt as though:
- the hierarchy with `TableScan` and `DataScan` (prior to this PR) would
then feel odd with a fully independent `IncrementalAppendScan`
- duplicated code is code smell, so I've gone with a refactor here. To note
that it's largely just moving code around than anything else! Let me know what
folks think or design suggestions here, very open to changes
(I realise this makes the diff here scary 😄 )
--
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]
