-
Notifications
You must be signed in to change notification settings - Fork 251
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
[Python] append column with a in-memory Pyarrow Table #338
Conversation
python/lance/_lib.pyx
Outdated
def append_column( | ||
self, | ||
field: Field, | ||
value: Union[Callable[[pyarrow.Table], pyarrow.Array], Expression], | ||
value: Union[Callable[[pyarrow.Table], pyarrow.Array], Expression, pyarrow.Table], |
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.
if we're merging the whole table then we can't call this append_column
.
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.
Should we create a dataset.merge
method? For now, limit it to horizontal concat. But later on we can allow for appending rows and columns in one shot? Pandas has this functionality, though the pandas merge API is way too complicated. wdyt?
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.
ok, i will make this a separate Merge function.
@@ -46,7 +49,9 @@ def value_func(x: pa.Table): | |||
assert x.column_names == ["a"] | |||
return pa.array([str(i) for i in x.column("a")]) | |||
|
|||
new_dataset = dataset.append_column(pa.field("c", pa.utf8()), value_func, columns=["a"]) | |||
new_dataset = dataset.append_column( | |||
value_func, field=pa.field("c", pa.utf8()), columns=["a"] |
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.
you know now that i'm looking at this API, having field
and columns
is really going to confuse people.
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.
How about change columns
to select
?
f58c13b
to
db6d628
Compare
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.
pending #337
074c77d
to
dc14a81
Compare
Closes #320