From 660154046f0efece3dcb1d761ab806cd7deb2d20 Mon Sep 17 00:00:00 2001 From: Mark Roberts Date: Tue, 30 Jul 2024 13:30:36 -0500 Subject: [PATCH 1/4] Add clang format check to workflows. --- .github/workflows/clang-format-check.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100755 .github/workflows/clang-format-check.yml diff --git a/.github/workflows/clang-format-check.yml b/.github/workflows/clang-format-check.yml new file mode 100755 index 0000000..4791bee --- /dev/null +++ b/.github/workflows/clang-format-check.yml @@ -0,0 +1,13 @@ +name: clang-format Check +on: [push, pull_request] +jobs: + formatting-check: + name: Formatting Check + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Run clang-format style check for C/C++/Protobuf programs. + uses: jidicula/clang-format-action@v4.13.0 + with: + clang-format-version: "13" + check-path: "mdio" From 6453a67ab67a55d524897addc17e435845f4414d Mon Sep 17 00:00:00 2001 From: Mark Roberts Date: Tue, 30 Jul 2024 18:43:52 +0000 Subject: [PATCH 2/4] Fix formatting issues. --- mdio/dataset.h | 112 ++++++++++++++++++++++++------------------------ mdio/variable.h | 19 ++++---- 2 files changed, 64 insertions(+), 67 deletions(-) diff --git a/mdio/dataset.h b/mdio/dataset.h index 394e0c1..b7ffecc 100644 --- a/mdio/dataset.h +++ b/mdio/dataset.h @@ -654,69 +654,67 @@ class Dataset { auto all_done_future = tensorstore::WaitAllFuture(futures); auto pair = tensorstore::PromiseFuturePair::Make(); - all_done_future.ExecuteWhenReady( - [promise = std::move(pair.promise), variables = std::move(variables), - metadata](tensorstore::ReadyFuture readyFut) { - mdio::VariableCollection collection; - mdio::coordinate_map coords; - std::unordered_map shape_size; - - for (const auto& fvar : variables) { - // we should have waited for this to be ready so it's not blocking - // ... - auto _var = fvar.result(); - if (!_var.ok()) { - promise.SetResult(_var.status()); - continue; - } - auto var = _var.value(); - - collection.add(var.get_variable_name(), std::move(var)); - // update coordinates if any: - auto meta = var.getMetadata(); - if (meta.contains("coordinates")) { - // Because of how Variable is set up, we need to break down a - // space delimited string to the vector - std::string coords_str = meta["coordinates"].get(); - std::vector coords_vec = - absl::StrSplit(coords_str, ' '); - coords[var.get_variable_name()] = coords_vec; - } + all_done_future.ExecuteWhenReady([promise = std::move(pair.promise), + variables = std::move(variables), + metadata](tensorstore::ReadyFuture + readyFut) { + mdio::VariableCollection collection; + mdio::coordinate_map coords; + std::unordered_map shape_size; + + for (const auto& fvar : variables) { + // we should have waited for this to be ready so it's not blocking + // ... + auto _var = fvar.result(); + if (!_var.ok()) { + promise.SetResult(_var.status()); + continue; + } + auto var = _var.value(); + + collection.add(var.get_variable_name(), std::move(var)); + // update coordinates if any: + auto meta = var.getMetadata(); + if (meta.contains("coordinates")) { + // Because of how Variable is set up, we need to break down a + // space delimited string to the vector + std::string coords_str = meta["coordinates"].get(); + std::vector coords_vec = absl::StrSplit(coords_str, ' '); + coords[var.get_variable_name()] = coords_vec; + } - auto domain = var.dimensions(); - auto shape = domain.shape().cbegin(); - for (const auto& label : domain.labels()) { - // FIXME check that if exists shape is the same ... - shape_size[label] = *shape; - ++shape; - } - } + auto domain = var.dimensions(); + auto shape = domain.shape().cbegin(); + for (const auto& label : domain.labels()) { + // FIXME check that if exists shape is the same ... + shape_size[label] = *shape; + ++shape; + } + } - std::vector keys; - std::vector values; + std::vector keys; + std::vector values; - keys.reserve(shape_size.size()); - values.reserve(shape_size.size()); - for (const auto& pair : shape_size) { - keys.push_back(std::move(pair.first)); - values.push_back(pair.second); - } + keys.reserve(shape_size.size()); + values.reserve(shape_size.size()); + for (const auto& pair : shape_size) { + keys.push_back(std::move(pair.first)); + values.push_back(pair.second); + } - auto dataset_domain = - tensorstore::IndexDomainBuilder<>(shape_size.size()) - .shape(values) - .labels(keys) - .Finalize(); + auto dataset_domain = tensorstore::IndexDomainBuilder<>(shape_size.size()) + .shape(values) + .labels(keys) + .Finalize(); - if (!dataset_domain.ok()) { - promise.SetResult(dataset_domain.status()); - return; - } + if (!dataset_domain.ok()) { + promise.SetResult(dataset_domain.status()); + return; + } - Dataset new_dataset{metadata, collection, coords, - dataset_domain.value()}; - promise.SetResult(std::move(new_dataset)); - }); + Dataset new_dataset{metadata, collection, coords, dataset_domain.value()}; + promise.SetResult(std::move(new_dataset)); + }); return pair.future; } diff --git a/mdio/variable.h b/mdio/variable.h index f773862..bfa34a7 100644 --- a/mdio/variable.h +++ b/mdio/variable.h @@ -843,15 +843,14 @@ class Variable { mdio::SliceDescriptor err; std::apply( [&](const auto&... desc) { - size_t idx = 0; - (( - [&] { - if (idx == preconditionStatus) { - err = desc; - } - idx++; - }() - ), ...); + size_t idx = 0; + (([&] { + if (idx == preconditionStatus) { + err = desc; + } + idx++; + }()), + ...); }, tuple_descs); return absl::InvalidArgumentError( @@ -1397,5 +1396,5 @@ Result> from_variable( variable.get_variable_name(), variable.get_long_name(), variable.getReducedMetadata(), std::move(labeled_array)}; } -}; // namespace mdio +}; // namespace mdio #endif // MDIO_VARIABLE_H_ From 15e16380a132bc2e2f3d464c4b857f3015aa8295 Mon Sep 17 00:00:00 2001 From: Mark Roberts Date: Wed, 31 Jul 2024 13:30:34 +0000 Subject: [PATCH 3/4] Bump clang-format version to 18. --- .github/workflows/clang-format-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clang-format-check.yml b/.github/workflows/clang-format-check.yml index 4791bee..f078a24 100755 --- a/.github/workflows/clang-format-check.yml +++ b/.github/workflows/clang-format-check.yml @@ -9,5 +9,5 @@ jobs: - name: Run clang-format style check for C/C++/Protobuf programs. uses: jidicula/clang-format-action@v4.13.0 with: - clang-format-version: "13" + clang-format-version: "18" check-path: "mdio" From 20b6ccda761560f8feeb25b8ef196c72df0c19a3 Mon Sep 17 00:00:00 2001 From: Mark Roberts Date: Wed, 31 Jul 2024 14:15:13 +0000 Subject: [PATCH 4/4] Formatting updates for clang-format v18 --- mdio/dataset.h | 112 ++++++++++++++++++++++++------------------------ mdio/variable.h | 2 +- 2 files changed, 58 insertions(+), 56 deletions(-) diff --git a/mdio/dataset.h b/mdio/dataset.h index b7ffecc..394e0c1 100644 --- a/mdio/dataset.h +++ b/mdio/dataset.h @@ -654,67 +654,69 @@ class Dataset { auto all_done_future = tensorstore::WaitAllFuture(futures); auto pair = tensorstore::PromiseFuturePair::Make(); - all_done_future.ExecuteWhenReady([promise = std::move(pair.promise), - variables = std::move(variables), - metadata](tensorstore::ReadyFuture - readyFut) { - mdio::VariableCollection collection; - mdio::coordinate_map coords; - std::unordered_map shape_size; - - for (const auto& fvar : variables) { - // we should have waited for this to be ready so it's not blocking - // ... - auto _var = fvar.result(); - if (!_var.ok()) { - promise.SetResult(_var.status()); - continue; - } - auto var = _var.value(); - - collection.add(var.get_variable_name(), std::move(var)); - // update coordinates if any: - auto meta = var.getMetadata(); - if (meta.contains("coordinates")) { - // Because of how Variable is set up, we need to break down a - // space delimited string to the vector - std::string coords_str = meta["coordinates"].get(); - std::vector coords_vec = absl::StrSplit(coords_str, ' '); - coords[var.get_variable_name()] = coords_vec; - } + all_done_future.ExecuteWhenReady( + [promise = std::move(pair.promise), variables = std::move(variables), + metadata](tensorstore::ReadyFuture readyFut) { + mdio::VariableCollection collection; + mdio::coordinate_map coords; + std::unordered_map shape_size; + + for (const auto& fvar : variables) { + // we should have waited for this to be ready so it's not blocking + // ... + auto _var = fvar.result(); + if (!_var.ok()) { + promise.SetResult(_var.status()); + continue; + } + auto var = _var.value(); + + collection.add(var.get_variable_name(), std::move(var)); + // update coordinates if any: + auto meta = var.getMetadata(); + if (meta.contains("coordinates")) { + // Because of how Variable is set up, we need to break down a + // space delimited string to the vector + std::string coords_str = meta["coordinates"].get(); + std::vector coords_vec = + absl::StrSplit(coords_str, ' '); + coords[var.get_variable_name()] = coords_vec; + } - auto domain = var.dimensions(); - auto shape = domain.shape().cbegin(); - for (const auto& label : domain.labels()) { - // FIXME check that if exists shape is the same ... - shape_size[label] = *shape; - ++shape; - } - } + auto domain = var.dimensions(); + auto shape = domain.shape().cbegin(); + for (const auto& label : domain.labels()) { + // FIXME check that if exists shape is the same ... + shape_size[label] = *shape; + ++shape; + } + } - std::vector keys; - std::vector values; + std::vector keys; + std::vector values; - keys.reserve(shape_size.size()); - values.reserve(shape_size.size()); - for (const auto& pair : shape_size) { - keys.push_back(std::move(pair.first)); - values.push_back(pair.second); - } + keys.reserve(shape_size.size()); + values.reserve(shape_size.size()); + for (const auto& pair : shape_size) { + keys.push_back(std::move(pair.first)); + values.push_back(pair.second); + } - auto dataset_domain = tensorstore::IndexDomainBuilder<>(shape_size.size()) - .shape(values) - .labels(keys) - .Finalize(); + auto dataset_domain = + tensorstore::IndexDomainBuilder<>(shape_size.size()) + .shape(values) + .labels(keys) + .Finalize(); - if (!dataset_domain.ok()) { - promise.SetResult(dataset_domain.status()); - return; - } + if (!dataset_domain.ok()) { + promise.SetResult(dataset_domain.status()); + return; + } - Dataset new_dataset{metadata, collection, coords, dataset_domain.value()}; - promise.SetResult(std::move(new_dataset)); - }); + Dataset new_dataset{metadata, collection, coords, + dataset_domain.value()}; + promise.SetResult(std::move(new_dataset)); + }); return pair.future; } diff --git a/mdio/variable.h b/mdio/variable.h index bfa34a7..bac93cb 100644 --- a/mdio/variable.h +++ b/mdio/variable.h @@ -1396,5 +1396,5 @@ Result> from_variable( variable.get_variable_name(), variable.get_long_name(), variable.getReducedMetadata(), std::move(labeled_array)}; } -}; // namespace mdio +}; // namespace mdio #endif // MDIO_VARIABLE_H_