Repository navigation
Conversation
|
📦 Python package built successfully!
|
`artifacts.store_dataframe(df, name)` writes a whole pandas or polars DataFrame to `deepnote_dataframes/<name>/<uuid>.arrow` in the project's filesystem, with `to_feather` / `write_ipc`, zstd falling back to lz4 and then uncompressed. A `manifest.json` names the current file and the one it replaced, and every write rewrites it. `artifacts.delete_dataframe(name)` deletes a name's directory. The function raises like the user's own `df.to_feather()` would, and returns None on a read-only mount. An interrupt stops the write and removes the partial file, except during the manifest update, which finishes first. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B441SpUqRn7QTzcnWxjNa5
tkislan
force-pushed
the
feat/artifacts-store-dataframe
branch
from
October 8, 2026 12:37
3d68fb4 to
62a4d0e
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #135 +/- ##
==========================================
+ Coverage 77.26% 77.90% +0.64%
==========================================
Files 115 118 +3
Lines 6589 6754 +165
Branches 961 979 +18
==========================================
+ Hits 5091 5262 +171
+ Misses 1186 1182 -4
+ Partials 312 310 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
5 of 7 tasks
The public API (store_dataframe, delete_dataframe) moves to deepnote_toolkit/artifacts/dataframes.py and is re-exported from the package, so `deepnote_toolkit.artifacts.store_dataframe` is unchanged. The writer and its manifest move alongside it, and their tests move to tests/unit/artifacts/. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
|
🚀 Review App Deployment Started
|
- Write manifest.json to a temporary file and rename it over the old one. s3fs uploads an empty object when a file is opened with O_TRUNC, so the in-place write left the name without a current frame during every store, and a failed write or upload left the manifest empty or naming a frame the call then deleted. - Raise a SIGINT held during the manifest update even when the update fails, so an I/O error can't swallow Stop. - delete_dataframe returns False on EROFS, as store_dataframe returns None, and documents the OSError it raises. - store_dataframe's docstring covers failures after the manifest names the new frame. - Drop the codec guard for an exception pyarrow never raises for zstd or lz4, with the two tests of that impossible state. - set_notebook_path() and the writer share config.resolve_notebook_root(), which keeps ipykernel out of the artifacts package's imports. - Move the Spark ANSI-mode toggle into a fixture. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
From 1.0 polars writes strings and binaries as Utf8View/BinaryView by default, which pyarrow before 15 and apache-arrow before 21.2.0 can't read, while the same data written by pandas reads there. From polars 1.2.1, where `pl.CompatLevel` is first exported, the writer passes `compat_level=pl.CompatLevel.oldest()`, so they are stored as large_string and large_binary. polars 1.0 to 1.2.0 still write views. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
- An s3fs rename copies the object and then deletes the source, so it can fail after the new manifest is already in place. store_frame then deleted the new frame, leaving the manifest naming a missing file. It now deletes the new frame only when the frame write or the manifest read fails; from write_manifest on, a failure keeps it, and a later write prunes it once it is a day old if no manifest names it. - With that, Stop at any point leaves a manifest that names a complete frame, so the SIGINT deferral around the manifest update is gone. Stop raises KeyboardInterrupt wherever it lands; the worst case is an unreferenced file. - Tests cover a rename that fails after the copy, a failed manifest read and an interrupted manifest write. The SIGINT test helpers and the tests of the held interrupt go with the deferral. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
Drop docstring paragraphs and comments that restate the code, describe behaviour the tests pin, or point at the RFC without a link. Keep the external facts: s3fs upload and rename semantics, polars and Python version quirks, the public store_dataframe and delete_dataframe contract. Fix the ones that were wrong: json raises RecursionError on deep nesting only before Python 3.12, polars has its own write_ipc errors, and the bad-frame helper returns the good call's manifest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
…here used The options dict was untyped, so mypy inferred dict[str, int] and only the repo's disabled assignment check hid the CompatLevel value going into it. The polars version is now parsed once. The comments above the constants moved to the two options they explain, each cut to one line: batches are capped so a page read stays cheap, which polars before 1.37 can't do, and the oldest compat level avoids the string and binary views older Arrow readers can't read. How readers treat batch sizes needs no comment, because an Arrow IPC file lists its batches and their row counts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
DataFrame.to_feather before pandas 2.1 refuses any index but a default RangeIndex and any non-string column label, so filtered, sorted, grouped, concatenated and transposed frames raised ValueError on pandas 2.0 and 1.5, which the declared range still allows on Python 3.10 and 3.11. pandas 2.1+ calls pyarrow's write_feather itself, so the files on the locked versions are byte-for-byte the same. GeoDataFrames, which geopandas writes with its own to_feather, now raise ArrowTypeError on their geometry column. The tests that faked or spied on to_feather now target write_feather, and the test helper that predicts refusals asks pyarrow too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
…esystem Two tests replaced read_manifest and _delete_replaced_frames wholesale, so no test made the filesystem calls inside them fail. read_manifest treating any OSError as "no manifest", or the cleanup swallowing OSError, kept the whole suite green. The first lets a transient EIO on manifest.json drop the frame that was current until then from the manifest, and delete it in the same call if it is more than a day old. The tests now fail Path.open on manifest.json and Path.unlink on the replaced frame, and both of those regressions fail them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
The statvfs probe ran before anything was written, so when the project root did not exist, which is the default outside Deepnote Cloud, store_dataframe and delete_dataframe both raised FileNotFoundError instead of creating the root or returning False. The probe also never caught anything the writes don't: on a read-only mount the first write (creating the name's directory, opening the frame file, or rmtree's first unlink) fails with EROFS before any conversion. store_frame now maps EROFS to None itself, and only while nothing is stored yet: creating the directory moved into the step that deletes the new frame on failure. EROFS from the manifest step on raises like any other error, because by then the new frame may already be current and None would say nothing was stored. The read-only test helper now refuses writes with EROFS instead of faking statvfs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
A query preview is a pandas DataFrame subclass, and the frames derived from it keep the subclass, so "needs a pandas or polars DataFrame, got DeepnoteQueryPreview" was false on its face and said nothing about why or what to do instead. The message now says a preview holds at most 100 rows of the query's result and points to the DataFrame output mode or pd.DataFrame(df). classify_frame still refuses previews, which the post-run hook relies on to skip them silently; store_dataframe only picks the message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
The 1,225-line test_artifacts.py is split into eight files by behaviour: storing, arguments, manifest and cleanup, file format, write failures, read-only mount, project root and delete_dataframe. The helpers and constants shared across them move to tests/unit/helpers/dataframe_storage.py under public names. No test changed: the same 150 tests are collected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
…nstants The test fixture root becomes tmp_project_root, _SAFE_ID and _FRAME_NAME become _SAFE_ID_REGEX and _FRAME_NAME_REGEX, and _FailingClose becomes _FailingCloseS3Upload. The Spark fixtures are typed as SparkSession, and store_frame loses the comment before the manifest step. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
Manifest is now a pydantic model of the whole manifest, and read_manifest and write_manifest both go through it. Reading used to check only file and previous; now pydantic checks every field's type, all of them required but previous, with strict types so "1", 1.5 and true are not ints. read_manifest then refuses a version other than 1, negative rows, and a file or previous that is not <uuid>.arrow. The name check stays on the read side because cleanup unlinks the replaced manifest's previous, and any code in the project can write the manifest. A refused manifest counts as absent, as before. Unknown keys are ignored. The written manifest is unchanged, previous still omitted when there is none. The pydantic_compat_helpers keep both pydantic 1 and 2 working; only known keys reach the model, since pydantic 1 raises TypeError on __pydantic_self__. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
A pytest.raises that names only a type passes on any error of that type, such as an OSError from a different step than the one under test. Each one now checks what it caught: the errno of an OSError, the message of a ValueError or TypeError, the identity of an injected error or interrupt, and the exact type where a subclass would also match. The read-only TypeError test is split so a SQL query preview's message is checked apart from an unsupported value's. rmtree's symbolic link error is compared by its args: newer Pythons (3.12.x and up) set its filename, and str() then no longer shows the message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
…cleared_query_preview function
…ditional _store_bad_frame_after_a_good_one took an optional expected_error and only wrapped the second store in pytest.raises when one was given. It now requires expected_error. _store_frame_after_a_good_one stores the second frame without expecting an error, and _store_a_good_frame holds the first store both share. test_frames_pyarrow_may_refuse_follow_what_pyarrow_does picks the helper by what pyarrow does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
stored_manifest validated nothing and returned a Dict[str, Any]. It now validates the written JSON with the Manifest model and returns it, so a field of the wrong type fails there, naming the field, and callers read typed attributes. "previous" not in X became X.previous is None: on a pydantic model, in checks against (name, value) pairs and would always pass. That no longer tells an omitted previous from null, and the model ignores unknown keys, so test_stores_pandas_frame_and_manifest checks the written keys from the raw JSON. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
When pyarrow accepts a version-dependent frame, the test only compared row counts. The uuid case has one row, as does the good frame stored before it, so a store that silently kept the previous frame passed. The test now also checks that the stored table is not the good frame. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
…fest The test claimed the store left nothing but checked only the frame files, so a store that wrote the manifest before the frame still passed it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
…ary file This is the only test where the rename fails, so nothing caught a write_manifest whose cleanup covered only the write and not the rename. That version leaves .manifest.json.*.tmp behind for good, because the cleanup prunes only <uuid>.arrow names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
… kind classify_frame mapped a frame to a "pandas" or "polars" string, and store_frame and _write_frame_file trusted that string without checking it against the frame. store_dataframe now checks the frame with the ocelots TypeGuards and passes on only the narrowed frame, typed as StorableFrame. _write_frame_file dispatches on it with isinstance and ends in assert_never. classify_frame also refused query previews while is_pandas_dataframe listed DeepnoteQueryPreview as pandas, and store_dataframe checked for a preview a second time to pick its error message. The refusal now lives only in store_dataframe. DeepnoteQueryPreview subclasses pd.DataFrame, so dropping it from is_pandas_dataframe changes no result. Stored files are byte-identical before and after, and every refused input raises the same TypeError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
The global config disables arg-type, the code mypy reports a non-exhaustive assert_never under, so a missing branch in _write_frame_file's isinstance chain went unreported. pandas has no stubs here, so PandasDF is Any and only the polars branch is checked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
pydantic ignores unknown keys by default on both v1 and v2. The filter to the model's fields only worked around v1's parse_obj, which calls cls(**obj): a key named __pydantic_self__, or a non-string key from a list of pairs, makes it raise TypeError rather than ValidationError. read_manifest now catches TypeError instead. The isinstance(dict) guard is gone too. On v2 every result is unchanged. On v1 a list of pairs is now honoured, still through every field and file-name check, and a __pydantic_self__ key counts as absent. The unknown-key-named-like-self case only covered the removed workaround, and on v2 it duplicates unknown-key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
…del class Callers got Any back, so mypy checked nothing they did with the result. The TypeVar is bound to BaseModel, so passing a class that isn't a model is a type-var error, which the repo's mypy config reports. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
…s and name checks 6c1fe41 removed the version, rows and file-name checks from read_manifest, so a manifest with version 2, negative rows, or a file or previous that isn't <uuid>.arrow is now honoured. The 12 cases that expected it to count as absent go, and so does the ../../data/x.csv sentinel only the traversal cases used. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
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
2 participants
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
Toolkit side of the Full Dataframe storage RFC (§3.4): notebook code can store a whole pandas or polars DataFrame under a name, and delete it. #131 builds on this to store a block's result automatically.
Changes
deepnote_toolkit/artifacts/: new package, exported fromdeepnote_toolkitlikeocelots.dataframes.py:store_dataframeanddelete_dataframe.dataframe_storage.py: the writer feat(dataframe-storage): store a block's result automatically #131 shares (checks, codec, write, manifest commit, cleanup).dataframe_storage_manifest.py: theManifestpydantic model, and reading, validating and writingmanifest.jsonthrough it.deepnote_toolkit/config.py:resolve_notebook_root(), moved out ofset_notebook_path.pyso frames use the same project root.tests/unit/artifacts/.Behaviour
<project root>/deepnote_dataframes/<name>/<uuid>.arrowplusmanifest.json(version,file,previous,written_at,rows,toolkit_version), which names the current file and the one it replaced. The UUID is the returned reference ID.write_feather, polars viawrite_ipc, in 64k-row batches (polars from 1.37). From polars 1.2.1, strings and binaries are written without the view types that pyarrow < 15 andapache-arrow< 21.2 can't read. Compression is zstd, else lz4, else none.EROFStoo) raises and keeps the new file, which the manifest may already name.previousframe and unreferenced<uuid>.arrowfiles older than a day, only inside the name's directory. A manifest counts as absent if it is over 4 KiB or not a JSON object, lacks a field (onlypreviousis optional) or has one of the wrong type, has aversionother than 1 or negativerows, or names anything but<uuid>.arrow. Unknown keys are ignored.EROFS):store_dataframereturnsNonebefore converting the frame;delete_dataframereturnsFalse.[A-Za-z0-9_-]{1,128}raisesValueError. Anything but a pandas or polars DataFrame, such as a PySpark frame or a polarsLazyFrame, raisesTypeError; a SQL query preview gets a message pointing to the DataFrame output mode. A frame pyarrow or polars can't serialize, such as a GeoDataFrame, raises their own error and keeps the previous frame.Not in this PR
The read endpoint and cleanup on project deletion. No feature flag or webapp error reports: a call is the user's own code (RFC §3.4.1, §3.4.3).