sungwy commented on code in PR #1036: URL: https://github.com/apache/iceberg-python/pull/1036#discussion_r1714372318
########## pyiceberg/table/__init__.py: ########## @@ -621,6 +621,13 @@ def delete(self, delete_filter: Union[str, BooleanExpression], snapshot_properti if not delete_snapshot.files_affected and not delete_snapshot.rewrites_needed: warnings.warn("Delete operation did not match any records") + def find_referenced_files(self, file_paths: List[str]) -> list[str]: Review Comment: We want to be intentional about introducing public methods. Since this is just used by `add_files`, could we make this method private? The type notation of `list` in the return type also needs to be `typing.List` until we deprecate python3.8 support: ```suggestion def _find_referenced_files(self, file_paths: List[str]) -> List[str]: ``` An alternative is to just keep this logic within `add_files`, since this method isn't reused and is just 2 or 3 lines of code -- 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