Visitar URL original
feat(core): expose optional Arrow converter by ibgreen · Pull Request #10957 · visgl/deck.gl · GitHub
Skip to content

feat(core): expose optional Arrow converter - #10957

Merged
ibgreen merged 4 commits into
masterfrom
codex/arrow-converter-prep
Oct 30, 2026
Merged

feat(core): expose optional Arrow converter#10957
ibgreen merged 4 commits into
masterfrom
codex/arrow-converter-prep

Conversation

@ibgreen

@ibgreen ibgreen commented Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Expose ArrowConverter directly through the optional @deck.gl/core/arrow subpath, preparing for the Scatterplot consumer in #10920.

  • Convert numeric scalars, positions, and RGB/RGBA colors locally using Apache Arrow and existing luma core/GPU-data packages, without @luma.gl/arrow.
  • Preserve chunk boundaries and Float64 residuals, optionally transform numeric rows and export validity vectors, and clean up partial uploads on failure.
  • Consume async iterators on demand, with cancellation and source cleanup.
  • Export the implementation directly, with no barrel exports or registration system.

Review regressions cover special output names such as __proto__ and nulls outside sliced list ranges.

Validation: all CI jobs pass, including WebGPU unit/render tests and the website build. Eleven converter tests, core type-checking, lint, and 31 precommit tests pass locally.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T00:23:49.972021Z ff19dee PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ibgreen ibgreen mentioned this pull request Oct 27, 2026
Merged
@greptile-apps

greptile-apps Bot commented Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge based on the checked code and updated callers in PR #10920.

Summary

Exposes ArrowConverter through the optional @deck.gl/core/arrow entry.

  • Arrow vectors convert into GPU-ready numeric, position, and color data.
  • Arrow record batches map named fields into aligned GPU columns.
  • Arrow streams convert one input at a time and can stop cleanly.

No new actionable issues were established. The earlier caller mismatch is addressed in PR #10920; that part of this assessment depends on the updated callers in that PR, not a fix contained here.

Checks were read from source; tests were not run.

Diagram

 
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Arrow vector or batch] --> B[ArrowConverter]
  B --> C[Convert each chunk]
  C --> D[Upload GPU buffers]
  D --> E[Caller-owned result]
  E --> F[Caller calls destroy]
  C -->|Failure| G[Release partial uploads]
Loading

Reviews (4) · Last reviewed commit: "test(core): adapt Arrow converter tests ..." · Reviewed by Greptile

greptile-apps[bot]
greptile-apps Bot reviewed Oct 27, 2026
View reviewed changes
Comment thread modules/core/src/lib/luma-upstream/arrow-converter.ts Resolved
Comment thread modules/core/src/lib/luma-upstream/arrow-converter.ts Resolved
cursor[bot]
cursor Bot reviewed Oct 27, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ff19dee. Configure here.

Comment thread modules/core/src/lib/luma-upstream/arrow-conversion-utils.ts Resolved
greptile-apps[bot]
greptile-apps Bot reviewed Oct 27, 2026
View reviewed changes
Comment thread modules/core/package.json Resolved
@coveralls

coveralls commented Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Coverage Status

No base build to compare — codex/arrow-converter-prep into master

@ibgreen
ibgreen force-pushed the codex/arrow-converter-prep branch from 8fe2c03 to a347204 Compare October 30, 2026 20:49
@ibgreen
ibgreen merged commit 77f6599 into master Oct 30, 2026
13 of 14 checks passed
@ibgreen
ibgreen deleted the codex/arrow-converter-prep branch October 30, 2026 21:21
chrisgervang
chrisgervang reviewed Nov 10, 2026
View reviewed changes

@chrisgervang chrisgervang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed a347204 after merge; follow-ups below. No P1s. One P2 question: whether @deck.gl/core/arrow is a public entry point, and if so where it is documented.

Labels: P1 settle before this is merged, P2 can follow, Non-blocking optional.

Comment thread modules/core/package.json
"import": "./dist/index.js",
"require": "./dist/index.cjs"
},
"./arrow": {

@chrisgervang chrisgervang Nov 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This publishes @deck.gl/core/arrow (ArrowConverter and its types) with an optional apache-arrow peer. Only src/arrow/README.md, titled "Arrow conversion prototype", describes it, and the class TSDoc says it is "intended for eventual luma.gl upstreaming".

  • On master, deck.gl layers import its ColumnSource type, and using-with-arrow.md sends applications to layer column props.

Is this entry point meant for applications? If yes, could it get an Experimental page under docs/api-reference/core, with the README's ownership rule (call destroy() after last use), and a whats-new bullet? If not, could using-with-arrow.md say it is internal to deck.gl packages?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chrisgervang chrisgervang chrisgervang left review comments
@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments
@cursor cursor[bot] cursor[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ibgreen @coveralls @chrisgervang