-
Notifications
You must be signed in to change notification settings - Fork 180
feat: Support concat(..., how="diagonal") for ibis
#3404
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
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
8caaf2a
feat(DRAFT): Support `concat(..., how="diagonal")` for `ibis`
dangotbanned f5c6407
refactor: Avoid poluting `CompliantNamespace`
dangotbanned 9db77b2
perf: piggyback off-of `ibis` validation
dangotbanned 7b5a3e1
test: Cover schema mismatch for `ibis`
dangotbanned cd4f50d
refactor: Tidy, optimize, document `align_diagonal`
dangotbanned ceceb5b
test: old `polars` compat
dangotbanned 75d5eac
test: try different message?
dangotbanned a55af2d
oops missed a spot
dangotbanned 2f76210
Merge branch 'main' into convert-concat-diagonal
dangotbanned a8e8388
test: Cover more than 2x table concat
dangotbanned 7225629
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 88db481
wtf pre-commit?
dangotbanned 759a1f6
perf: Get super lazy, incorporate @FBruzzesi suggestion
dangotbanned f2901b6
Apply doc suggestion
dangotbanned 25a698b
Apply `concat` suggestion
dangotbanned f5a6fdc
losing braincells
dangotbanned File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
(#3404 (comment))
@FBruzzesi okay this led to discovering a bug (but not in any of the new code or your suggestion)
ibisis the only backend that doesn't guarantee the order ofunion.I found that out by using the 3-tabled version of the test:
That fails for
ibis, but it turns out"vertical"fails too:Show test
Show error
I suppose I'm stuck with testing two tables for now then 😂I've added (a8e8388), but should probably follow this up with another issue.
(We (and
polars) don't document that it is ordered, but we do test for it andpolars.unionwas recently introduced for unordered)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.
For a moment I thought order of columns, and I panicked sooo much 🤯
No guarantee in row order makes sense. You can add an index column and sort by such