Skip to content

feat: Support concat(..., how="diagonal") for ibis#3404

Merged
dangotbanned merged 16 commits intomainfrom
convert-concat-diagonal
Jan 17, 2026
Merged

feat: Support concat(..., how="diagonal") for ibis#3404
dangotbanned merged 16 commits intomainfrom
convert-concat-diagonal

Conversation

@dangotbanned
Copy link
Member

@dangotbanned dangotbanned commented Jan 15, 2026

Description

While reviewing #3398, one option I raised would be to adopt how polars handles how="diagonal".

Whether we do/don't can be decided on later, but by adding support for ibis now ...

Important

All backends support concat={"diagonal", "vertical"}

Related issues

Managed to cause **33!** yells from pyright originally
after aligning, all frames are guaranteed to have the same column names in the same order
checking the dtypes *too*, when `ibis` will handle this already seems excessive
Surprised we don't have tests for this
@dangotbanned dangotbanned added enhancement New feature or request tests ibis Issue is related to ibis backend labels Jan 15, 2026
) -> None:
if "ibis" in str(constructor):
request.applymarker(pytest.mark.xfail)
def test_concat_diagonal(constructor: Constructor) -> None:
Copy link
Member Author

@dangotbanned dangotbanned Jan 15, 2026

Choose a reason for hiding this comment

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

(#3404 (comment))

I do think there's something here, I just wanna experiment a bit (and add more tests) ❤️

@FBruzzesi okay this led to discovering a bug (but not in any of the new code or your suggestion)

ibis is the only backend that doesn't guarantee the order of union.

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

def test_concat_vertical_bigger(constructor: Constructor) -> None:
    data_1 = {"a": [1, 2], "b": [3, 4], "c": [0, None]}
    data_2 = {"a": [5, 6], "b": [0, None], "c": [7, 8]}
    data_3 = {"a": [0, None], "b": [9, 10], "c": [11, 12]}
    expected = {
        "a": [1, 2, 5, 6, 0, None],
        "b": [3, 4, 0, None, 9, 10],
        "c": [0, None, 7, 8, 11, 12],
    }
    df_1 = nw.from_native(constructor(data_1)).lazy()
    df_2 = nw.from_native(constructor(data_2)).lazy()
    df_3 = nw.from_native(constructor(data_3)).lazy()
    result = nw.concat([df_1, df_2, df_3], how="vertical")
    assert_equal_data(result, expected)

Show error

E               AssertionError: Mismatch at index 0, key a: 0 != 1
E               Expected: {'a': [1, 2, 5, 6, 0, None], 'b': [3, 4, 0, None, 9, 10], 'c': [0, None, 7, 8, 11, 12]}
E               Got: {'a': [0, None, 5, 6, 1, 2], 'b': [9, 10, 0, None, 3, 4], 'c': [11, 12, 7, 8, 0, None]}

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 and polars.union was recently introduced for unordered)

Copy link
Member

Choose a reason for hiding this comment

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

ibis is the only backend that doesn't guarantee the order of union.

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

@dangotbanned dangotbanned marked this pull request as ready for review January 15, 2026 16:29
@FBruzzesi FBruzzesi self-requested a review January 15, 2026 16:57
Copy link
Member

@FBruzzesi FBruzzesi left a comment

Choose a reason for hiding this comment

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

Thanks @dangotbanned - I left just one comment for the ibis implementation.

On the protocol side I think we debated enough yesterday 😇

dangotbanned and others added 3 commits January 16, 2026 16:03
Co-authored-by: Francesco Bruzzesi <42817048+FBruzzesi@users.noreply.github.com>
Co-authored-by: Francesco Bruzzesi <42817048+FBruzzesi@users.noreply.github.com>
Copy link
Member

@FBruzzesi FBruzzesi left a comment

Choose a reason for hiding this comment

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

It's a yes from me 🚀

@dangotbanned dangotbanned merged commit 96fcaf2 into main Jan 17, 2026
32 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request ibis Issue is related to ibis backend tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants