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.

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

Jump to bottom

pretty-print: add pretty print #469

Merged
merged 6 commits into from
Feb 8, 2023
Merged

pretty-print: add pretty print #469

merged 6 commits into from
Feb 8, 2023

Conversation

AsadullahFarooqi
Copy link
Contributor

haven't tested these prints because I haven't seen any query example

@AsadullahFarooqi
Copy link
Contributor Author

AsadullahFarooqi commented Jan 27, 2023 •
edited
Loading

this branch/PR will be rebased after this PR #468

@AsadullahFarooqi AsadullahFarooqi force-pushed the feature/pretty-print branch 4 times, most recently from 61eba28 to 34f7a5a Compare January 28, 2023 19:57
@AsadullahFarooqi
Copy link
Contributor Author

AsadullahFarooqi commented Jan 28, 2023 •
edited
Loading

Screenshot from 2023-01-29 01-00-52
The PR is ready IMO @changhiskhan @eddyxu
I think #477 issue is genuine the limit can't go over 1024 and even if I put some lower limit. it still panics rather than a graceful ending

@AsadullahFarooqi
Copy link
Contributor Author

just noticed the commit msg pattern you guys use. let me quickly update that

@eddyxu
Copy link
Contributor

eddyxu commented Jan 30, 2023

Since we included many arrow-* submodules, curious about what would be the difference in the final binary size to include arrow.

@AsadullahFarooqi
Copy link
Contributor Author

Since we included many arrow-* submodules, curious about what would be the difference in the final binary size to include arrow.

by binary you mean just the lq bin or anything else as well?

eddyxu
eddyxu reviewed Jan 30, 2023
View reviewed changes
rust/src/bin/lq.rs Outdated
let mut total_num_of_cols: usize = 0;
for b in batch.iter() {
total_num_of_rows += b.num_rows();
total_num_of_cols += b.num_columns();
Copy link
Contributor

Choose a reason for hiding this comment

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

The columns could be the same amount batches. We could just print num of columns from the first batch.

For the number of rows, let's wait for #495 to get in?

The rest LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right the cols would be same from the 1st batch

@AsadullahFarooqi
Copy link
Contributor Author

@changhiskhan @eddyxu plz merge this pr

@eddyxu eddyxu merged commit 1c1b654 into lancedb:main Feb 8, 2023
@eddyxu
Copy link
Contributor

eddyxu commented Feb 8, 2023

Thanks @AsadullahFarooqi for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eddyxu eddyxu eddyxu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AsadullahFarooqi @eddyxu