rdblue commented on code in PR #6525:
URL: https://github.com/apache/iceberg/pull/6525#discussion_r1067584072
##########
python/pyiceberg/typedef.py:
##########
@@ -79,6 +84,10 @@ def __missing__(self, key: K) -> V:
class StructProtocol(Protocol): # pragma: no cover
"""A generic protocol used by accessors to get and set at positions of an
object"""
+ @abstractmethod
+ def set_record_schema(self, record_schema: StructType) -> None:
Review Comment:
One thing that I didn't think about when we were discussing where to put
this method is that this is can be called multiple times if we don't pass the
schema to the constructor, allowing callers to change the record schema. We
assume that it will only be set once in the readers, but this makes the API
quite strange. I can read with one schema then set a different schema, and then
the behavior is unclear.
Because of that, I'd prefer to move this method to our `PydanticStruct`
class instead of putting it here. That way we control all of the
implementations and it doesn't matter. And we will have the ability to change
it later if needed.
--
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]
