kbendick commented on code in PR #4717: URL: https://github.com/apache/iceberg/pull/4717#discussion_r882945870
########## python/src/iceberg/table/partitioning.py: ########## @@ -14,8 +14,13 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from typing import Dict, Iterable, List, Tuple + +from iceberg.schema import Schema from iceberg.transforms import Transform +_PARTITION_DATA_ID_START: int = 1000 + class PartitionField: Review Comment: I'll hold off on commenting too much until I have a chance to look into the `pydantic` project as well as look at the other PR. My first ask would be how many dependencies are we bringing in if we add `pydantic`? I know that some folks were concerned about adding very many external python dependencies, so as not to conflict with their own dependencies, but if the benefit is very large I'm not personally opposed to it (I believe it was somebody / some group from Netflix that originally requested we keep the number of required dependencies down). But validation, either via a library or via a common pattern we settle on, is something that would be very beneficial. -- 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]
