From e34b6a6af59fabe68b59c6475829799706d70b26 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Sun, 14 Jan 2024 22:11:33 -0500 Subject: [PATCH 1/3] CMake Updates --- CMakeLists.txt | 4 ++++ pantab/src/CMakeLists.txt | 1 + 2 files changed, 5 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index f5fdb0b4..0efa51cb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -49,4 +49,8 @@ if (PANTAB_USE_SANITIZERS) add_link_options(-fsanitize=address -fsanitize=undefined) endif() +find_program(CLANG_TIDY_EXE NAMES "clang-tidy") +set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "-checks=-*,modernize-*") + + add_subdirectory(pantab/src) diff --git a/pantab/src/CMakeLists.txt b/pantab/src/CMakeLists.txt index 4fc881ad..ee17c12b 100644 --- a/pantab/src/CMakeLists.txt +++ b/pantab/src/CMakeLists.txt @@ -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) From 34084a8f6c3101be9f8c36d2b2d52865ffc02a53 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Sun, 14 Jan 2024 22:25:45 -0500 Subject: [PATCH 2/3] fix clang-tidy warnings --- pantab/src/pantab.cpp | 55 +++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/pantab/src/pantab.cpp b/pantab/src/pantab.cpp index a96d5352..a015086e 100644 --- a/pantab/src/pantab.cpp +++ b/pantab/src/pantab.cpp @@ -26,8 +26,8 @@ using Dtype = std::tuple; 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: " + @@ -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_]; @@ -246,10 +246,11 @@ class TimestampInsertHelper : public InsertHelper { } }; -static std::unique_ptr -makeInsertHelper(std::shared_ptr inserter, - struct ArrowArray *chunk, struct ArrowSchema *schema, - struct ArrowError *error, int64_t column_position) { +static auto makeInsertHelper(std::shared_ptr inserter, + struct ArrowArray *chunk, + struct ArrowSchema *schema, + struct ArrowError *error, int64_t column_position) + -> std::unique_ptr { // TODO: we should provide the full dtype here not just format string, so // boolean fields can determine whether they are bit or byte masks @@ -387,7 +388,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}); } @@ -404,7 +405,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"); @@ -435,13 +436,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(); } @@ -450,7 +451,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(); } @@ -459,7 +460,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()); if (value.isNull()) { @@ -470,7 +471,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(); } @@ -479,7 +480,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(); } @@ -498,7 +499,7 @@ class DateReadHelper : public ReadHelper { }; template 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(); } @@ -526,7 +527,8 @@ template class DatetimeReadHelper : public ReadHelper { } }; -static std::unique_ptr makeReadHelper(hyperapi::SqlType sqltype) { +static auto makeReadHelper(hyperapi::SqlType sqltype) + -> std::unique_ptr { if ((sqltype == hyperapi::SqlType::smallInt()) || (sqltype == hyperapi::SqlType::integer()) || (sqltype == hyperapi::SqlType::bigInt())) { @@ -548,7 +550,8 @@ static std::unique_ptr 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()) { @@ -584,8 +587,8 @@ using PandasDtypes = std::vector; /// because the former detects a schema from the hyper Result object /// which does not hold nullability information /// -std::tuple -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 { std::vector> result; hyperapi::HyperProcess hyper{ hyperapi::Telemetry::DoNotSendUsageDataToTableau}; @@ -620,9 +623,9 @@ read_from_hyper_query(const std::string &path, const std::string &query) { return std::make_tuple(result, columnNames, pandasDtypes); } -std::tuple -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 { std::vector> result; hyperapi::HyperProcess hyper{ hyperapi::Telemetry::DoNotSendUsageDataToTableau}; @@ -661,7 +664,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"), From 6a56199c9551544c6a354b84b4bbabdec99279b0 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Tue, 16 Jan 2024 22:05:46 -0500 Subject: [PATCH 3/3] fix find_program --- CMakeLists.txt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0efa51cb..d3a64826 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -50,7 +50,10 @@ if (PANTAB_USE_SANITIZERS) endif() find_program(CLANG_TIDY_EXE NAMES "clang-tidy") -set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "-checks=-*,modernize-*") - +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)