moomindani commented on PR #3491: URL: https://github.com/apache/iceberg-python/pull/3491#issuecomment-4774455852
Thanks for splitting this out — the read path looks behavior-preserving and the separation is clean. For the write side, we'd like to coordinate before building on top of this: once it lands we're planning to rebase #3474 (the DV writer, currently `PuffinWriter` → to be renamed `DeletionVectorWriter`) on top of it, moving the writer along with its `_serialize_bitmaps` helper and the DV-specific constants into `deletion_vector.py` and reusing `MAX_JAVA_SIGNED` / `PROPERTY_REFERENCED_DATA_FILE` from here. Given that, would it make sense to decide now whether the write/serialize path should live on `DeletionVector` itself (mirroring `from_puffin_file` / `to_vector` with a serialize counterpart) or as a separate `DeletionVectorWriter` class? Our preference would be to mirror the read-side split — keep the DV bitmap serialization on `DeletionVector` (as the counterpart to `from_puffin_file` / `to_vector`) and leave the generic Puffin-file assembly as a format-level writer in `puffin.py`, so the write path scales to the future NDV blob the same way the read path now does. That said, we're happy either way. -- 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]
