Skip to content

fix: schema conversion, add conversion test cases#3468

Merged
ion-elgreco merged 1 commit intodelta-io:mainfrom
ion-elgreco:fix/schema-conversion
May 28, 2025
Merged

fix: schema conversion, add conversion test cases#3468
ion-elgreco merged 1 commit intodelta-io:mainfrom
ion-elgreco:fix/schema-conversion

Conversation

@ion-elgreco
Copy link
Collaborator

Description

With the refactor I introduced a minor bug, but it's only fixable with an upstream change in arro3, kylebarron/arro3#334. cc @kylebarron

I already have the new code stashed locally, so once arro3 is released I will push those changes and it should work ✨

The conversion test cases will highlight the issue for lists that lose the name, nullability and metadata ^^

@github-actions github-actions bot added the binding/python Issues for the Python package label May 25, 2025
@codecov
Copy link

codecov bot commented May 25, 2025

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.23%. Comparing base (226c397) to head (616b352).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
python/src/lib.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3468      +/-   ##
==========================================
+ Coverage   71.21%   71.23%   +0.01%     
==========================================
  Files         149      149              
  Lines       44825    44825              
  Branches    44825    44825              
==========================================
+ Hits        31922    31930       +8     
+ Misses      10831    10828       -3     
+ Partials     2072     2067       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

rtyler
rtyler previously approved these changes May 27, 2025
Copy link
Collaborator

@roeap roeap left a comment

Choose a reason for hiding this comment

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

just some general comments we may want to look at for 1.0.

Copy link
Collaborator

Choose a reason for hiding this comment

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

aside: could we mark this as experimental as well for 1.0?

Not sure where this will end up, but there are two things 1) we can get rid of much of the flattening code by using RecordBatch::normalize(".") and 2) we currently generate this from add actions, but should move to using record batches we read from the log directly.

The new approach might be to expose a schema and the batches ? in the end though I think this is how we should expose actions, but may want to change the schema of the returned data.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But we haven't for all the previous releases, wouldn't that be strange to mark it experimental now. I think we can still deprecate changes beyond 1.0.0

@ion-elgreco ion-elgreco added this pull request to the merge queue May 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch May 28, 2025
Signed-off-by: Ion Koutsouris <15728914+ion-elgreco@users.noreply.github.com>
@ion-elgreco ion-elgreco force-pushed the fix/schema-conversion branch from bed60fc to 616b352 Compare May 28, 2025 09:25
@ion-elgreco ion-elgreco enabled auto-merge May 28, 2025 09:26
@ion-elgreco ion-elgreco added this pull request to the merge queue May 28, 2025
Merged via the queue into delta-io:main with commit 36b4b77 May 28, 2025
24 of 25 checks passed
@ion-elgreco ion-elgreco deleted the fix/schema-conversion branch May 28, 2025 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

binding/python Issues for the Python package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants