Fokko commented on code in PR #6525:
URL: https://github.com/apache/iceberg/pull/6525#discussion_r1069363001
##########
python/pyiceberg/avro/reader.py:
##########
@@ -238,41 +249,50 @@ def skip(self, decoder: BinaryDecoder) -> None:
return self.option.skip(decoder)
-class StructProtocolReader(Reader):
- create_struct: Callable[[], StructProtocol]
- fields: Tuple[Tuple[Optional[int], Reader], ...]
+class StructReader(Reader):
+ field_readers: Tuple[Tuple[Optional[int], Reader], ...]
+ create_struct: Type[StructProtocol]
+ struct: Optional[StructType]
- def __init__(self, fields: Tuple[Tuple[Optional[int], Reader], ...],
create_struct: Callable[[], StructProtocol]):
- self.create_struct = create_struct
- self.fields = fields
-
- def create_or_reuse(self, reuse: Optional[StructProtocol]) ->
StructProtocol:
- if reuse:
- return reuse
- else:
- return self.create_struct()
+ def __init__(
+ self,
+ field_readers: Tuple[Tuple[Optional[int], Reader], ...],
+ create_struct: Optional[Type[StructProtocol]] = None,
Review Comment:
I'm second-guessing this. I think this comes at a great expense. In
retrospect, I'd rather add the `__init__(struct: StructType)` to the
StructProtocol as I don't think that's a weird thing to do. Now we accept
constructors of StructProtocols and just arbitrary functions (I had to change
the signature to `Callable[..., StructProtcol]`, so you can literally almost
put everything in there, as long as it returns a StructProtocol. We're not
using this flexibility today, and I'm not sure if we're going to need it
tomorrow.
By just requiring the StructProtocol, we can check if it adheres to the
protocol when we construct the reader, and everything should be good (mypy will
tell you if you're not). Now, this is changed to inspecting the signature when
we actually start reading and you'll end up with a runtime error.
--
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]
