[GitHub] [iceberg] cccs-eric edited a comment on pull request #3989: Python: MapType missing required methods and visit() not supporting MapType

[GitBox <gi...@apache.org>]

cccs-eric edited a comment on pull request #3989:
URL: https://github.com/apache/iceberg/pull/3989#issuecomment-1025658560


   @rdblue I've changed the `asMapType` method to `as_map_type` as requested.  I simply wanted to highlight in my previous [comment](https://github.com/apache/iceberg/pull/3989#discussion_r793977478) that this method was part of the `Type` interface.  But I'm totally fine with that change.
   
   As for the `NotImplementedError` being raised in the visitor, did you want to also get that fixed?  I had a look and there is a code gap [here](https://github.com/apache/iceberg/blob/master/python_legacy/iceberg/api/types/type_util.py#L172), but I tried to remove catch clauses in the visitor to see if any test would break and everything passed.  So we are obviously missing test coverage in that area, so I think a fix for this should go in another PR, or simply be moved to the new python refactoring.
   
   Let me know if want anything else added to this PR. 


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org