sungwy commented on PR #3491: URL: https://github.com/apache/iceberg-python/pull/3491#issuecomment-4795426793
> 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. Hi @moomindani - thanks for flagging this, and for thinking about how the two paths line up. I'm leaning the same way you are: keep the DV bitmap serialization on `DeletionVector` as the counterpart to `from_puffin_file` / `to_vector`, and leave the generic Puffin assembly (agreed `PuffinWriter` sounds like a good name) as a format-level writer in `puffin.py`. That keeps the write path scaling to the future NDV blob the same way the read path now does. Happy to look at #3474 once this lands. Does that work for you? -- 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]
