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

Clang tidy #224

Merged
merged 5 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,11 @@ if (PANTAB_USE_SANITIZERS)
add_link_options(-fsanitize=address -fsanitize=undefined)
endif()

find_program(CLANG_TIDY_EXE NAMES "clang-tidy")
if (CLANG_TIDY_EXE_FOUND)
set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "-checks=-*,modernize-*")
else()
message("Could not find clang-tidy installation - checks disabled")
endif()

add_subdirectory(pantab/src)
1 change: 1 addition & 0 deletions pantab/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ target_link_libraries(pantab
PRIVATE Tableau::tableauhyperapi-cxx
PRIVATE nanoarrow
)
set_target_properties(pantab PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}")
set_target_properties(nanoarrow
PROPERTIES POSITION_INDEPENDENT_CODE
ON)
Expand Down
55 changes: 29 additions & 26 deletions pantab/src/pantab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ using Dtype = std::tuple<int, int, std::string, std::string>;

enum TimeUnit { SECOND, MILLI, MICRO, NANO };

static hyperapi::SqlType hyperTypeFromArrowSchema(struct ArrowSchema *schema,
ArrowError *error) {
static auto hyperTypeFromArrowSchema(struct ArrowSchema *schema,
ArrowError *error) -> hyperapi::SqlType {
struct ArrowSchemaView schema_view;
if (ArrowSchemaViewInit(&schema_view, schema, error) != 0) {
throw std::runtime_error("Issue converting to hyper type: " +
Expand Down Expand Up @@ -71,7 +71,7 @@ class InsertHelper {
: inserter_(std::move(inserter)), chunk_(chunk), schema_(schema),
error_(error), column_position_(column_position) {}

virtual ~InsertHelper() {}
virtual ~InsertHelper() = default;

void Init() {
struct ArrowSchema *child_schema = schema_->children[column_position_];
Expand Down Expand Up @@ -250,10 +250,11 @@ class TimestampInsertHelper : public InsertHelper {
}
};

static std::unique_ptr<InsertHelper>
makeInsertHelper(std::shared_ptr<hyperapi::Inserter> inserter,
struct ArrowArray *chunk, struct ArrowSchema *schema,
struct ArrowError *error, int64_t column_position) {
static auto makeInsertHelper(std::shared_ptr<hyperapi::Inserter> inserter,
struct ArrowArray *chunk,
struct ArrowSchema *schema,
struct ArrowError *error, int64_t column_position)
-> std::unique_ptr<InsertHelper> {
// TODO: we should provide the full dtype here not just format string, so
// boolean fields can determine whether they are bit or byte masks

Expand Down Expand Up @@ -391,7 +392,7 @@ void write_to_hyper(
names_vec.push_back(name);

// Almost all arrow types are nullable
hyper_columns.push_back(hyperapi::TableDefinition::Column{
hyper_columns.emplace_back(hyperapi::TableDefinition::Column{
name, hypertype, hyperapi::Nullability::Nullable});
}

Expand All @@ -408,7 +409,7 @@ void write_to_hyper(
struct ArrowArray chunk;
int errcode;
while ((errcode = stream->get_next(stream.get(), &chunk) == 0) &&
chunk.release != NULL) {
chunk.release != nullptr) {
const int nrows = chunk.length;
if (nrows < 0) {
throw std::runtime_error("Unexpected array length < 0");
Expand Down Expand Up @@ -439,13 +440,13 @@ void write_to_hyper(

class ReadHelper {
public:
ReadHelper() {}
virtual ~ReadHelper() {}
virtual nb::object Read(const hyperapi::Value &) { return nb::none(); }
ReadHelper() = default;
virtual ~ReadHelper() = default;
virtual auto Read(const hyperapi::Value &) -> nb::object = 0;
};

class IntegralReadHelper : public ReadHelper {
nb::object Read(const hyperapi::Value &value) {
auto Read(const hyperapi::Value &value) -> nb::object override {
if (value.isNull()) {
return nb::none();
}
Expand All @@ -454,7 +455,7 @@ class IntegralReadHelper : public ReadHelper {
};

class FloatReadHelper : public ReadHelper {
nb::object Read(const hyperapi::Value &value) {
auto Read(const hyperapi::Value &value) -> nb::object override {
if (value.isNull()) {
return nb::none();
}
Expand All @@ -463,7 +464,7 @@ class FloatReadHelper : public ReadHelper {
};

class BooleanReadHelper : public ReadHelper {
nb::object Read(const hyperapi::Value &value) {
auto Read(const hyperapi::Value &value) -> nb::object override {
// TODO: bool support added in nanobind >= 1..9.0
// return nb::bool_(value.get<bool>());
if (value.isNull()) {
Expand All @@ -474,7 +475,7 @@ class BooleanReadHelper : public ReadHelper {
};

class StringReadHelper : public ReadHelper {
nb::object Read(const hyperapi::Value &value) {
auto Read(const hyperapi::Value &value) -> nb::object override {
if (value.isNull()) {
return nb::none();
}
Expand All @@ -483,7 +484,7 @@ class StringReadHelper : public ReadHelper {
};

class DateReadHelper : public ReadHelper {
nb::object Read(const hyperapi::Value &value) {
auto Read(const hyperapi::Value &value) -> nb::object override {
if (value.isNull()) {
return nb::none();
}
Expand All @@ -502,7 +503,7 @@ class DateReadHelper : public ReadHelper {
};

template <bool TZAware> class DatetimeReadHelper : public ReadHelper {
nb::object Read(const hyperapi::Value &value) {
auto Read(const hyperapi::Value &value) -> nb::object override {
if (value.isNull()) {
return nb::none();
}
Expand Down Expand Up @@ -530,7 +531,8 @@ template <bool TZAware> class DatetimeReadHelper : public ReadHelper {
}
};

static std::unique_ptr<ReadHelper> makeReadHelper(hyperapi::SqlType sqltype) {
static auto makeReadHelper(hyperapi::SqlType sqltype)
-> std::unique_ptr<ReadHelper> {
if ((sqltype == hyperapi::SqlType::smallInt()) ||
(sqltype == hyperapi::SqlType::integer()) ||
(sqltype == hyperapi::SqlType::bigInt())) {
Expand All @@ -552,7 +554,8 @@ static std::unique_ptr<ReadHelper> makeReadHelper(hyperapi::SqlType sqltype) {
throw nb::type_error(("cannot read sql type: " + sqltype.toString()).c_str());
}

static std::string pandasDtypeFromHyper(const hyperapi::SqlType &sqltype) {
static auto pandasDtypeFromHyper(const hyperapi::SqlType &sqltype)
-> std::string {
if (sqltype == hyperapi::SqlType::smallInt()) {
return "int16[pyarrow]";
} else if (sqltype == hyperapi::SqlType::integer()) {
Expand Down Expand Up @@ -588,8 +591,8 @@ using PandasDtypes = std::vector<std::string>;
/// because the former detects a schema from the hyper Result object
/// which does not hold nullability information
///
std::tuple<ResultBody, ColumnNames, PandasDtypes>
read_from_hyper_query(const std::string &path, const std::string &query) {
auto read_from_hyper_query(const std::string &path, const std::string &query)
-> std::tuple<ResultBody, ColumnNames, PandasDtypes> {
std::vector<std::vector<nb::object>> result;
hyperapi::HyperProcess hyper{
hyperapi::Telemetry::DoNotSendUsageDataToTableau};
Expand Down Expand Up @@ -624,9 +627,9 @@ read_from_hyper_query(const std::string &path, const std::string &query) {
return std::make_tuple(result, columnNames, pandasDtypes);
}

std::tuple<ResultBody, ColumnNames, PandasDtypes>
read_from_hyper_table(const std::string &path, const std::string &schema,
const std::string &table) {
auto read_from_hyper_table(const std::string &path, const std::string &schema,
const std::string &table)
-> std::tuple<ResultBody, ColumnNames, PandasDtypes> {
std::vector<std::vector<nb::object>> result;
hyperapi::HyperProcess hyper{
hyperapi::Telemetry::DoNotSendUsageDataToTableau};
Expand Down Expand Up @@ -665,7 +668,7 @@ read_from_hyper_table(const std::string &path, const std::string &schema,
return std::make_tuple(result, columnNames, pandasDtypes);
}

NB_MODULE(pantab, m) {
NB_MODULE(pantab, m) { // NOLINT
m.def("write_to_hyper", &write_to_hyper, nb::arg("dict_of_exportable"),
nb::arg("path"), nb::arg("table_mode"))
.def("read_from_hyper_query", &read_from_hyper_query, nb::arg("path"),
Expand Down
Loading