Skip to content
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

feat: port create_table to the async python API and the remote rust API #1031

Merged
merged 8 commits into from
Feb 29, 2024

Conversation

westonpace
Copy link
Contributor

@westonpace westonpace commented Feb 27, 2024

I've also started ASYNC_MIGRATION.MD to keep track of the breaking changes from sync to async python.

nodejs/lancedb/native.d.ts Show resolved Hide resolved
Comment on lines 1799 to 1818
>>> table = db.create_table("my_table", data=[{"vector": [1.1, 1.2], "b": 2}])
>>> table.head()
pyarrow.Table
vector: fixed_size_list<item: float>[2]
child 0, item: float
b: int64
----
vector: [[[1.1,1.2]]]
b: [[2]]

Can append new data with [Table.add()][lancedb.table.Table.add].

>>> table.add([{"vector": [0.5, 1.3], "b": 4}])

Can query the table with [Table.search][lancedb.table.Table.search].

>>> table.search([0.4, 0.4]).select(["b"]).to_pandas()
b vector _distance
0 4 [0.5, 1.3] 0.82
1 2 [1.1, 1.2] 1.13
Copy link
Contributor

Choose a reason for hiding this comment

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

These examples probably need await now, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or is this left as a TODO?

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'm going to leave this as a TODO. My rationale is that some of these examples need other capabilities (e.g. this one needs add and so it'll be easier to do the doc tests all at once at the end.

#1044

Comment on lines 46 to 53
let mode = if mode == "create" {
CreateTableMode::Create
} else if mode == "overwrite" {
CreateTableMode::Overwrite
} else {
// TODO: allow users to pass in open options
CreateTableMode::exist_ok(|builder| builder)
};
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: seems like a good use for match?

Suggested change
let mode = if mode == "create" {
CreateTableMode::Create
} else if mode == "overwrite" {
CreateTableMode::Overwrite
} else {
// TODO: allow users to pass in open options
CreateTableMode::exist_ok(|builder| builder)
};
match mode {
"create" => CreateTableMode::Create,
"overwrite" => CreateTableMode::Overwrite,
_ => CreateTableMode::exist_ok(|builder| builder)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched to match and put in a helper function to share with create_empty_table. I also updated the node version to behave more similarly to the python version.

.post(&format!("/v1/table/{}/create", options.name))
.body(data_buffer)
.header(CONTENT_TYPE, ARROW_STREAM_CONTENT_TYPE)
.header("x-request-id", "na")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is for tracing purposes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was a UUID. It's no longer used by LanceDb Cloud. However, LDBC returns a deserialize error if the field is missing. We should fully remove it from LDBC and then we can remove it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems worth a comment.

… async in python. Added mode/existOk to typescript create_table. Changed countRows in TS to return number instead of bigint
@westonpace westonpace marked this pull request as ready for review February 29, 2024 19:18
@westonpace westonpace merged commit 2a02d13 into lancedb:main Feb 29, 2024
17 checks passed
raghavdixit99 pushed a commit to raghavdixit99/lancedb that referenced this pull request Apr 5, 2024
…PI (lancedb#1031)

I've also started `ASYNC_MIGRATION.MD` to keep track of the breaking
changes from sync to async python.
raghavdixit99 pushed a commit to raghavdixit99/lancedb that referenced this pull request Apr 5, 2024
…PI (lancedb#1031)

I've also started `ASYNC_MIGRATION.MD` to keep track of the breaking
changes from sync to async python.
westonpace added a commit that referenced this pull request Apr 5, 2024
…PI (#1031)

I've also started `ASYNC_MIGRATION.MD` to keep track of the breaking
changes from sync to async python.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants