Skip to content

add concat #137

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.

Sign up for GitHub

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 15 commits into from
May 15, 2023
Merged

add concat #137

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
46eb08e
draft concat
Apr 17, 2023
fa16046
note about strictness
Apr 17, 2023
e9ef937
note ordering
Apr 26, 2023
2e941cc
Merge remote-tracking branch 'upstream/main' into concat
Apr 27, 2023
d323908
note that order is indeed preservedd
Apr 27, 2023
1caf2e4
Merge remote-tracking branch 'upstream/main' into concat
Apr 27, 2023
ce68605
lint
Apr 27, 2023
eed4ef4
fixup first/last rendering
Apr 27, 2023
0277fcc
move concat to __init__
Apr 28, 2023
2827cd6
Update spec/API_specification/dataframe_api/dataframe_object.py
MarcoGorelli May 2, 2023
71bb209
Revert "Update spec/API_specification/dataframe_api/dataframe_object.py"
May 2, 2023
dbbad7e
Update spec/API_specification/dataframe_api/__init__.py
MarcoGorelli May 10, 2023
3ba1a4b
Update spec/API_specification/dataframe_api/__init__.py
MarcoGorelli May 11, 2023
b7cedb7
Merge remote-tracking branch 'upstream/main' into concat
MarcoGorelli May 15, 2023
869f858
post-merge fixup
MarcoGorelli May 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions spec/API_specification/dataframe_api/dataframe_object.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,22 @@ def get_column_names(self) -> Sequence[str]:
"""
...

def concat(self, other: Sequence[DataFrame]) -> DataFrame:
"""
Concatenate current and other DataFrames vertically.
Copy link
Member

Choose a reason for hiding this comment

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

How about mentioning that column names must match? And must ordering match too?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We may want to document any row ordering guarantees or lack thereof as well. I.E. does this guarantee that for df1.concat([df2, df3]) that the row order is df1 rows, then df2 rows, then df3 rows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I presume so - or should that not be guaranteed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe ANSI SQL doesn't guarantee any ordering of UNION ALL (which is the dataframe concat equivalent) unless there's also an ORDER BY clause (sort). For example, postgres doesn't guarantee any maintained ordering in a UNION ALL operation. So DataFrame APIs backed by a relational database would possibly struggle to meet that guarantee.

In general though, the often desired behavior is to maintain the order based on the passed in objects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i wasn't aware - ok thanks have added a note

Copy link
Collaborator

Choose a reason for hiding this comment

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

I.E. Ibis (https://github.com/ibis-project/ibis) backed by basically any of its backends (https://github.com/ibis-project/ibis#backends) where the DataFrame(s) in question are the result of running joins.

There's no guarantees made in output ordering for the joins as well as no guarantees made in output ordering for the concatenation. It could potentially be worked around by doing something like using SQL ROW_NUMBER() functions to get ordering of the input tables to the concatenate call and then sort by that generated column afterwards, but that's somewhat expensive.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need resolution here before we can merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we've got Column.__getitem__, what does that return for an unordered column?

I don't know what the resolution is here, this discussion's a bit abstract for me - what do you suggest?

Copy link
Collaborator

Choose a reason for hiding this comment

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

we've got Column.__getitem__, what does that return for an unordered column?

Potentially a random row or set of rows with no guarantee about determinism.

Maybe we can be a bit looser in the language and leave it how it currently is which is a bit underdefined which basically makes it implementation defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok - anything you'd like changed, or ok to merge?


To concatenate horizontally, please use ``insert``.

Parameters
----------
other : Sequence[DataFrame]
DataFrames to concatenate with.

Returns
-------
DataFrame
"""

def __eq__(self, other: DataFrame | Scalar) -> DataFrame:
"""
Compare for equality.
Expand Down