smaheshwar-pltr commented on PR #2031: URL: https://github.com/apache/iceberg-python/pull/2031#issuecomment-2960876386
> Going over the PR, I'm not sure if we want to copy the whole class hierarchy from Java, as this does not feel very Python in my opinion. Thanks for taking a look, @Fokko - that makes sense. I think abstract classes used in this PR's way achieve append scan functionalities nicely and without duplication (see the added tests in `tests/integration/test_reads.py`), and in line with existing code: the previous `TableScan` has [abstract methods](https://github.com/apache/iceberg-python/blob/dea5f77709fbfca844705f25973e7416e62837b8/pyiceberg/table/__init__.py#L1568-L1604). But the hierarchy indeed may be confusing, maybe even more than usual given I've tried to not introduce breaks, and it's true that rearrangement bloats the diff 😄. Maybe this PR's logic, of planning files for an append scan, APIs, and tests can still be reviewed? -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
