Repository navigation
Conversation
… DataFrames store_dataframe wrote frames that could be loaded whole but not served a page at a time. The manifest had no per-batch row counts, batches were capped by rows only, and compression could fall back to lz4. - Write pandas and polars frames through one pyarrow IPC path that cuts the record batches itself and records their row counts in the manifest as batch_rows. - Cap batches at about 8 MiB as well as 65,536 rows, sizing them from the average row, and merge small chunks into whole batches. - Compress with zstd, else not at all. - Raise the manifest cap to 64 KiB, and refuse a frame whose manifest would exceed it. - Refuse polars Object columns, nested ones included. Rechunk chunked Categoricals on polars before 1.35.1. Fixes #141 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VNWoySEbnsryRwTGKUydNx
|
📦 Python package built successfully!
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #142 +/- ##
==========================================
+ Coverage 78.17% 78.30% +0.12%
==========================================
Files 120 120
Lines 6837 6876 +39
Branches 990 997 +7
==========================================
+ Hits 5345 5384 +39
Misses 1182 1182
Partials 310 310
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@coderabbitai ignore
Summary
Fixes #141.
store_dataframe, added in 2.9.0, writes frames that can be loaded whole but not served a page at a time. Its manifest lists no per-batch row counts, batches are capped by rows only, and compression can fall back to lz4. With this PR the toolkit cuts the record batches itself and records each batch's row count in the manifest asbatch_rows. Batches are capped by bytes as well as rows, lz4 is gone, and the manifest cap rises to 64 KiB.Changes
deepnote_toolkit/artifacts/_dataframe_storage.py:pa.Table.from_pandas(df, preserve_index=None), the conversionwrite_featherdid.df.to_arrow(), withcompat_level=oldestfrom 1.2.1 as before.pa.ipc.new_file, and the row count of each batch is returned.clamp(8 MiB / average row bytes, 1, 65,536).nbytes, because every batch writes its own copy of each column and chunk, even ones that share buffers.min(nbytes, get_total_buffer_size()). The file holds the dictionary once, whilenbytescounts it once per chunk.TypeError, including Object inside a List, Array or Struct.to_arrow()would export each value's memory address.deepnote_toolkit/artifacts/_dataframe_storage_manifest.py:batch_rowsis a new required field, androwsis its sum.encode_manifestraisesValueErrorpast the cap, before anything is replaced.deepnote_toolkit/artifacts/dataframes.py: docstring.tests/unit/artifacts/.Behaviour changes
batch_rowscounts as absent. The next store of that name doesn't keep the old file asprevious, and the old file is deleted once it is a day old, like any unreferenced frame. Frames stored by 2.9.0 can't be served a page at a time until they are stored again.ValueErrorand keeps the previous frame. 2.9.0 never had this limit; without the guard, such a frame would get a manifest nothing can read.TypeErrorinstead of polars'ComputeError. On polars 1.39,write_ipcaborted the process for aList(Object)column; storing one now raises.ArrowInvalidfromto_arrow().write_ipcused to write a file pyarrow couldn't read.to_arrow()converts the whole frame at once, and copies string columns from views tolarge_string. On a 1.4 GiB frame of 10M strings the peak rose by 1.47 GiB, against 0.35 GiB withwrite_ipc. pandas is unchanged, becausefrom_pandasalways copied the whole frame.Known limits
write_ipc, becauseto_arrow()costs about 0.09 ms per chunk.Testing
Storage tests: 127 pass on Python 3.13 (pyarrow 24.0.0, pandas 2.2.3, polars 1.39.3) and on Python 3.10 (pyarrow 16.1.0, pandas 2.1.4).
Full unit suite: 1,406 passed and 10 skipped, with no new warnings.
Mutation check: each of 24 mutants of the new code fails at least one test. They include:
Version checks in throwaway environments: pyarrow 10.0.1, 13.0.0, 16.1.0 and 24.0.0 with pandas 1.5.3 to 2.2.3, and polars 0.20.31 to 1.39.3.
End to end: a server-side page reader, with its manifest cap raised to 64 KiB, served every page at pageSize 1000 and 100 for frames written by this branch. The frames covered:
nbytesestimate, 0 of 400 pages were served);pd.concat([df] * 5),pl.concat([df] * 10)and aliased columns;Wide frames hit the reader's per-page value limit only at pageSize 1000.
🤖 Generated with Claude Code
https://claude.ai/code/session_01VNWoySEbnsryRwTGKUydNx