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

CLI to inspect lance dataset #231

Merged
merged 16 commits into from
Oct 13, 2022
Merged

CLI to inspect lance dataset #231

merged 16 commits into from
Oct 13, 2022

Conversation

eddyxu
Copy link
Contributor

@eddyxu eddyxu commented Oct 12, 2022

Writing in C++ to reuse lance code

@eddyxu eddyxu marked this pull request as ready for review October 13, 2022 00:18
@eddyxu eddyxu requested a review from changhiskhan October 13, 2022 01:51
@eddyxu eddyxu self-assigned this Oct 13, 2022
@eddyxu eddyxu added enhancement New feature or request c++ C++ issues labels Oct 13, 2022
Copy link
Contributor

@changhiskhan changhiskhan left a comment

Choose a reason for hiding this comment

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

nice!

@@ -28,6 +28,7 @@
#include <string>

#include "lance/arrow/file_lance.h"
#include "lance/arrow/utils.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

is this used in io.cc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@@ -21,6 +21,7 @@
#include <memory>

#include "lance/arrow/file_lance_ext.h"
#include "lance/format/format.pb.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

where is this used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. it was due to some forward declaration during dev.

@@ -702,4 +702,24 @@ pb::Field::Type Field::GetNodeType() const {
}
}

void Print(const Field& field, const std::string& path, int indent = 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

would it make sense to print the storage type for extension types here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure can do

@eddyxu eddyxu merged commit e1cf7b9 into main Oct 13, 2022
@eddyxu eddyxu deleted the lei/cli branch October 13, 2022 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ C++ issues enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants