samredai commented on a change in pull request #3988:
URL: https://github.com/apache/iceberg/pull/3988#discussion_r793769057
##########
File path: python_legacy/iceberg/api/types/conversions.py
##########
@@ -90,11 +99,7 @@ def to_byte_buffer(type_id, value):
@staticmethod
def from_byte_buffer(type_var, buffer_var):
- return Conversions.internal_from_byte_buffer(type_var.type_id,
buffer_var)
-
- @staticmethod
- def internal_from_byte_buffer(type_id, buffer_var):
try:
- return Conversions.from_byte_buff_mapping.get(type_id)(type_id,
buffer_var)
+ return
Conversions.from_byte_buff_mapping.get(type_var.type_id)(type_var, buffer_var)
except KeyError:
- raise TypeError("Cannot deserialize Type: %s" % type_id)
+ raise TypeError("Cannot deserialize Type: %s" % type_var)
Review comment:
Ah I see, that makes sense. With the write `__str__` for `type_var` I
think using `type_var` alone would make sense, assuming that the type is also
visible. The main thing I think is that since it's a `KeyError`, the error
should make it obvious what the key was. Even including both feels ok to me.
`f"Cannot deserialize {type_var.type_id}: {type_var}"`
--
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]
