-
Notifications
You must be signed in to change notification settings - Fork 171
feat(@deepnote/convert): add per-block Jupyter export with convertBlockToJupyterCell #215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThe package version increments to 2.2.0. A previously internal per-block converter function is renamed, promoted to the public API as 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #215 +/- ##
=======================================
Coverage 91.57% 91.57%
=======================================
Files 41 41
Lines 2041 2041
Branches 630 630
=======================================
Hits 1869 1869
Misses 172 172 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/convert/src/deepnote-to-jupyter.ts (1)
183-183: TODO noted.Consider tracking
outputs_referenceas a follow-up issue.Want me to open an issue for this?
🤖 Fix all issues with AI agents
In `@packages/convert/src/deepnote-to-jupyter.test.ts`:
- Around line 293-441: Add edge-case tests that cover blocks with empty content
and blocks with missing/undefined metadata: create DeepnoteBlock fixtures where
content is '' and where metadata is omitted or set to undefined, call
convertBlockToJupyterCell (and optionally convertBlocksToJupyterNotebook for a
list case) and assert expected behavior (cell.source becomes '' or empty
array/string as appropriate, cell.metadata exists and preserves deepnote_*
fields only when present, and no crashes occur). Include references to
convertBlockToJupyterCell, convertBlocksToJupyterNotebook, and DeepnoteBlock in
the new tests.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/convert/package.jsonpackages/convert/src/deepnote-to-jupyter.test.tspackages/convert/src/deepnote-to-jupyter.tspackages/convert/src/index.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Place test files next to source files with.test.tsor.test.tsxextension
Use strict type checking in TypeScript files
Prefer type safety over convenience in TypeScript
Useconstfor immutable values
Avoidanytypes - use proper type definitions
Follow Biome's rules (configured inbiome.json)
Use literal keys instead of bracket notation when possible
Prefer single quotes for strings (except when avoiding escapes)
Keep code clean and readable following Biome standards
Files:
packages/convert/src/index.tspackages/convert/src/deepnote-to-jupyter.test.tspackages/convert/src/deepnote-to-jupyter.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Use Vitest as the testing framework
Follow existing test patterns in the codebase (seepackages/blocks/src/blocks/*.test.ts)
Test edge cases, error handling, and special characters
Files:
packages/convert/src/deepnote-to-jupyter.test.ts
🔇 Additional comments (9)
packages/convert/package.json (1)
3-3: Version bump looks correct.Minor version increment (2.1.3 → 2.2.0) aligns with adding
convertBlockToJupyterCellas a new, backward-compatible public API.packages/convert/src/index.ts (1)
5-10: Export addition is clean.
convertBlockToJupyterCellproperly added to public API.JupyterCelltype already exported at line 89 for typing the return value.packages/convert/src/deepnote-to-jupyter.ts (3)
52-52: Clean delegation.
convertBlocksToJupyterNotebooknow calls the new public per-block converter, maintaining single responsibility.
127-146: Good documentation.JSDoc with streaming example clearly communicates the intended use case.
147-188: Implementation looks solid.Property access via
inchecks handles the union type correctly. Metadata spread with explicit overrides ensures custom fields take precedence.packages/convert/src/deepnote-to-jupyter.test.ts (4)
293-314: Good test structure.Covers cell type, source, execution count, outputs, and metadata mapping.
368-399: Nice parity test.Ensures per-block conversion matches batch conversion output—prevents drift between the two APIs.
401-420: Input block test covers metadata preservation.Verifies
deepnote_variable_nameanddeepnote_variable_valuepass through.
422-440: Execution timing coverage is thorough.Tests
executionStartedAt,executionFinishedAt, andcontentHashpreservation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| "name": "@deepnote/convert", | ||
| "version": "2.1.3", | ||
| "version": "2.2.0", | ||
| "description": "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, i just include bumps in these to save time on waiting for approvals, release immediately after it gets merged.
Worst case scenario - there will be a conflict in this line and i'll need to update it when it happens
Summary
convertBlockToJupyterCellfunction for streaming export of individual Deepnote blocks to Jupyter cellsJupyterCelltype for TypeScript consumersMotivation
Consumers need to export large notebooks to Jupyter format in a streaming fashion—processing blocks one at a time rather than loading the entire notebook into memory.
Usage
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.