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

Reply via email to