From e546f7d23e849d94e45ea03b8895421d89fb5abd Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Wed, 18 Sep 2024 15:38:03 -0400 Subject: [PATCH 1/2] Respond to PR comments. --- CMakeLists.txt | 3 + src/CMakeLists.txt | 6 +- src/driver/CMakeLists.txt | 110 +++++++++++++------------- tests/CMakeLists.txt | 4 +- tests/driver/CMakeLists.txt | 154 ++++++++++++++++++------------------ 5 files changed, 138 insertions(+), 139 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2925e3f8..0419a48b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,6 +19,9 @@ set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) +option(NOTEST "Disable all tests" OFF) +option(BUILD_ACQUIRE_DRIVER_ZARR "Build the Acquire Zarr driver" ON) + add_subdirectory(src) add_subdirectory(tests) add_subdirectory(examples) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f9a3b42a..31635d84 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,3 +1,3 @@ -set(CMAKE_POSITION_INDEPENDENT_CODE ON) - -add_subdirectory(driver) +if (BUILD_ACQUIRE_DRIVER_ZARR) + add_subdirectory(driver) +endif () diff --git a/src/driver/CMakeLists.txt b/src/driver/CMakeLists.txt index e223b5db..5b72798a 100644 --- a/src/driver/CMakeLists.txt +++ b/src/driver/CMakeLists.txt @@ -1,61 +1,57 @@ -option(BUILD_ACQUIRE_DRIVER_ZARR "Build the Acquire Zarr driver" ON) +if (NOT TARGET acquire-core-logger) + add_subdirectory(${CMAKE_SOURCE_DIR}/acquire-common/acquire-core-libs ${CMAKE_CURRENT_BINARY_DIR}/acquire-core-libs) +endif () -if (BUILD_ACQUIRE_DRIVER_ZARR) - if (NOT TARGET acquire-core-logger) - add_subdirectory(${CMAKE_SOURCE_DIR}/acquire-common/acquire-core-libs ${CMAKE_CURRENT_BINARY_DIR}/acquire-core-libs) - endif () +set(tgt acquire-driver-zarr) +add_library(${tgt} MODULE + common/dimension.hh + common/dimension.cpp + common/thread.pool.hh + common/thread.pool.cpp + common/s3.connection.hh + common/s3.connection.cpp + common/utilities.hh + common/utilities.cpp + writers/sink.hh + writers/sink.creator.hh + writers/sink.creator.cpp + writers/file.sink.hh + writers/file.sink.cpp + writers/s3.sink.hh + writers/s3.sink.cpp + writers/array.writer.hh + writers/array.writer.cpp + writers/zarrv2.array.writer.hh + writers/zarrv2.array.writer.cpp + writers/zarrv3.array.writer.hh + writers/zarrv3.array.writer.cpp + writers/blosc.compressor.hh + writers/blosc.compressor.cpp + zarr.hh + zarr.cpp + zarr.v2.hh + zarr.v2.cpp + zarr.v3.hh + zarr.v3.cpp + zarr.driver.c +) - set(tgt acquire-driver-zarr) - add_library(${tgt} MODULE - common/dimension.hh - common/dimension.cpp - common/thread.pool.hh - common/thread.pool.cpp - common/s3.connection.hh - common/s3.connection.cpp - common/utilities.hh - common/utilities.cpp - writers/sink.hh - writers/sink.creator.hh - writers/sink.creator.cpp - writers/file.sink.hh - writers/file.sink.cpp - writers/s3.sink.hh - writers/s3.sink.cpp - writers/array.writer.hh - writers/array.writer.cpp - writers/zarrv2.array.writer.hh - writers/zarrv2.array.writer.cpp - writers/zarrv3.array.writer.hh - writers/zarrv3.array.writer.cpp - writers/blosc.compressor.hh - writers/blosc.compressor.cpp - zarr.hh - zarr.cpp - zarr.v2.hh - zarr.v2.cpp - zarr.v3.hh - zarr.v3.cpp - zarr.driver.c - ) +target_include_directories(${tgt} PRIVATE + $ +) - target_include_directories(${tgt} PRIVATE - $ - ) +target_enable_simd(${tgt}) +target_link_libraries(${tgt} PRIVATE + acquire-core-logger + acquire-core-platform + acquire-device-kit + acquire-device-properties + blosc_static + nlohmann_json::nlohmann_json + miniocpp::miniocpp +) +set_target_properties(${tgt} PROPERTIES + MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>" +) - target_enable_simd(${tgt}) - target_link_libraries(${tgt} PRIVATE - acquire-core-logger - acquire-core-platform - acquire-device-kit - acquire-device-properties - blosc_static - nlohmann_json::nlohmann_json - miniocpp::miniocpp - ) - set_target_properties(${tgt} PROPERTIES - MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>" - ) - - install(TARGETS ${tgt} LIBRARY DESTINATION lib) -endif () \ No newline at end of file +install(TARGETS ${tgt} LIBRARY DESTINATION lib) \ No newline at end of file diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 87878b37..d10a9f44 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,5 +1,7 @@ if (${NOTEST}) message(STATUS "Skipping test targets") else () - add_subdirectory(driver) + if (BUILD_ACQUIRE_DRIVER_ZARR) + add_subdirectory(driver) + endif () endif () diff --git a/tests/driver/CMakeLists.txt b/tests/driver/CMakeLists.txt index 1fd661a2..b2ea8266 100644 --- a/tests/driver/CMakeLists.txt +++ b/tests/driver/CMakeLists.txt @@ -1,87 +1,85 @@ -if (BUILD_ACQUIRE_DRIVER_ZARR) - set(NOTEST "TRUE") - add_subdirectory(${CMAKE_SOURCE_DIR}/acquire-common ${CMAKE_CURRENT_BINARY_DIR}/acquire-common) - set(NOTEST "FALSE") +set(NOTEST "TRUE") +add_subdirectory(${CMAKE_SOURCE_DIR}/acquire-common ${CMAKE_CURRENT_BINARY_DIR}/acquire-common) +set(NOTEST "FALSE") - # - # PARAMETERS - # - set(project acquire-driver-zarr) # CMAKE_PROJECT_NAME gets overridden if this is a subtree of another project +# +# PARAMETERS +# +set(project acquire-driver-zarr) # CMAKE_PROJECT_NAME gets overridden if this is a subtree of another project - # - # Tests - # - set(tests - list-devices - unit-tests - get - get-meta - get-set-get - external-metadata-with-whitespace-ok - restart-stopped-zarr-resets-threadpool - repeat-start - metadata-dimension-sizes - write-zarr-v2-raw - write-zarr-v2-raw-chunk-size-larger-than-frame-size - write-zarr-v2-raw-with-even-chunking - write-zarr-v2-raw-with-even-chunking-and-rollover - write-zarr-v2-raw-with-ragged-chunking - write-zarr-v2-with-lz4-compression - write-zarr-v2-with-zstd-compression - write-zarr-v2-compressed-with-chunking - write-zarr-v2-compressed-with-chunking-and-rollover - write-zarr-v2-raw-multiscale - write-zarr-v2-raw-multiscale-with-trivial-tile-size - write-zarr-v2-compressed-multiscale - write-zarr-v2-to-s3 - multiscales-metadata - write-zarr-v3-raw - write-zarr-v3-raw-with-ragged-sharding - write-zarr-v3-raw-chunk-exceeds-array - write-zarr-v3-compressed - write-zarr-v3-raw-multiscale - write-zarr-v3-to-s3 - ) +# +# Tests +# +set(tests + list-devices + unit-tests + get + get-meta + get-set-get + external-metadata-with-whitespace-ok + restart-stopped-zarr-resets-threadpool + repeat-start + metadata-dimension-sizes + write-zarr-v2-raw + write-zarr-v2-raw-chunk-size-larger-than-frame-size + write-zarr-v2-raw-with-even-chunking + write-zarr-v2-raw-with-even-chunking-and-rollover + write-zarr-v2-raw-with-ragged-chunking + write-zarr-v2-with-lz4-compression + write-zarr-v2-with-zstd-compression + write-zarr-v2-compressed-with-chunking + write-zarr-v2-compressed-with-chunking-and-rollover + write-zarr-v2-raw-multiscale + write-zarr-v2-raw-multiscale-with-trivial-tile-size + write-zarr-v2-compressed-multiscale + write-zarr-v2-to-s3 + multiscales-metadata + write-zarr-v3-raw + write-zarr-v3-raw-with-ragged-sharding + write-zarr-v3-raw-chunk-exceeds-array + write-zarr-v3-compressed + write-zarr-v3-raw-multiscale + write-zarr-v3-to-s3 +) - foreach (name ${tests}) - set(tgt "${project}-${name}") - add_executable(${tgt} ${name}.cpp) - target_compile_definitions(${tgt} PUBLIC "TEST=\"${tgt}\"") - set_target_properties(${tgt} PROPERTIES - MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>" - ) - target_include_directories(${tgt} PRIVATE "${CMAKE_CURRENT_LIST_DIR}/../") - target_link_libraries(${tgt} - acquire-core-logger - acquire-core-platform - acquire-video-runtime - nlohmann_json::nlohmann_json - miniocpp::miniocpp - ) +foreach (name ${tests}) + set(tgt "${project}-${name}") + add_executable(${tgt} ${name}.cpp) + target_compile_definitions(${tgt} PUBLIC "TEST=\"${tgt}\"") + set_target_properties(${tgt} PROPERTIES + MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>" + ) + target_include_directories(${tgt} PRIVATE "${CMAKE_CURRENT_LIST_DIR}/../") + target_link_libraries(${tgt} + acquire-core-logger + acquire-core-platform + acquire-video-runtime + nlohmann_json::nlohmann_json + miniocpp::miniocpp + ) - add_test(NAME test-${tgt} COMMAND ${tgt}) - set_tests_properties(test-${tgt} PROPERTIES LABELS "anyplatform;acquire-driver-zarr") - endforeach () + add_test(NAME test-${tgt} COMMAND ${tgt}) + set_tests_properties(test-${tgt} PROPERTIES LABELS "anyplatform;acquire-driver-zarr") +endforeach () - # - # Copy driver to tests - # - list(POP_FRONT tests onename) +# +# Copy driver to tests +# +list(POP_FRONT tests onename) - foreach (driver - acquire-driver-common - acquire-driver-zarr +foreach (driver + acquire-driver-common + acquire-driver-zarr +) + add_custom_target(${project}-copy-${driver}-for-tests + COMMAND ${CMAKE_COMMAND} -E copy + $ + $ + DEPENDS ${driver} + COMMENT "Copying ${driver} to $" ) - add_custom_target(${project}-copy-${driver}-for-tests - COMMAND ${CMAKE_COMMAND} -E copy - $ - $ - DEPENDS ${driver} - COMMENT "Copying ${driver} to $" - ) - foreach (name ${tests}) - add_dependencies(${tgt} ${project}-copy-${driver}-for-tests) - endforeach () + foreach (name ${tests}) + add_dependencies(${tgt} ${project}-copy-${driver}-for-tests) endforeach () -endif () \ No newline at end of file +endforeach () \ No newline at end of file From 42a494055bf9a835d12dd944b7bfca3135cb1134 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Fri, 20 Sep 2024 09:45:17 -0400 Subject: [PATCH 2/2] Respond to PR comments. --- include/acquire.zarr.h | 223 ++++------------------------------------- include/zarr.types.h | 10 +- 2 files changed, 20 insertions(+), 213 deletions(-) diff --git a/include/acquire.zarr.h b/include/acquire.zarr.h index eb287663..cbbbaf86 100644 --- a/include/acquire.zarr.h +++ b/include/acquire.zarr.h @@ -1,5 +1,4 @@ -#ifndef H_ACQUIRE_ZARR_V0 -#define H_ACQUIRE_ZARR_V0 +#pragma once #include "zarr.types.h" @@ -8,7 +7,19 @@ extern "C" { #endif - typedef struct ZarrStreamSettings_s ZarrStreamSettings; + typedef struct ZarrStreamSettings_s + { + ZarrS3Settings s3_settings; + ZarrCompressionSettings compression_settings; + ZarrDimensionProperties* dimensions; + size_t dimension_count; + char* store_path; + char* custom_metadata; + ZarrDataType data_type; + ZarrVersion version; + bool multiscale; + } ZarrStreamSettings; + typedef struct ZarrStream_s ZarrStream; /** @@ -22,7 +33,7 @@ extern "C" * @param level The log level. * @return ZarrStatus_Success on success, or an error code on failure. */ - ZarrStatus Zarr_set_log_level(ZarrLogLevel level); + ZarrStatusCode Zarr_set_log_level(ZarrLogLevel level); /** * @brief Get the log level for the Zarr API. @@ -35,191 +46,7 @@ extern "C" * @param status The status code. * @return A human-readable status message. */ - const char* Zarr_get_error_message(ZarrStatus status); - - /** - * @brief Create a Zarr stream settings struct. - * @return A pointer to the Zarr stream settings struct, or NULL on failure. - */ - ZarrStreamSettings* ZarrStreamSettings_create(); - - /** - * @brief Destroy a Zarr stream settings struct. - * @details This function frees the memory allocated for the Zarr stream - * settings struct. - * @param[in] settings The Zarr stream settings struct. - */ - void ZarrStreamSettings_destroy(ZarrStreamSettings* settings); - - /** - * @brief Copy a Zarr stream settings struct. - * @param[in] settings The Zarr stream settings struct to copy. - * @return A copy of the Zarr stream settings struct. - */ - ZarrStreamSettings* ZarrStreamSettings_copy( - const ZarrStreamSettings* settings); - - /** - * @brief Set store path and S3 settings for the Zarr stream. - * @param[in, out] settings - * @param[in] store_path The store path for the Zarr stream. Directory path - * when acquiring to the filesystem, key prefix when acquiring to S3. - * @param[in] bytes_of_store_path The length of @p store_path in bytes, - * including the null terminator. - * @param[in] s3_settings Optional S3 settings. If NULL, the store path is - * assumed to be a directory path. - * @return ZarrStatus_Success on success, or an error code on failure. - */ - ZarrStatus ZarrStreamSettings_set_store(ZarrStreamSettings* settings, - const char* store_path, - size_t bytes_of_store_path, - const ZarrS3Settings* s3_settings); - - /** - * @brief Set the data type, compressor, codec, compression_settings level, - * and shuffle for the Zarr stream. - * @param[in, out] settings The Zarr stream settings struct. - * @param[in] compression_settings The compression_settings settings. - * @return ZarrStatus_Success on success, or an error code on failure. - */ - ZarrStatus ZarrStreamSettings_set_compression( - ZarrStreamSettings* settings, - const ZarrCompressionSettings* compression_settings); - - /** - * @brief Set the data type for the Zarr stream. - * @param[in, out] settings The Zarr stream settings struct. - * @param[in] data_type The data type. - * @return ZarrStatus_Success on success, or an error code on failure. - */ - ZarrStatus ZarrStreamSettings_set_data_type(ZarrStreamSettings* settings, - ZarrDataType data_type); - - /** - * @brief Reserve space for dimensions in the Zarr stream settings struct. - * @detail *Must* precede calls to ZarrStreamSettings_set_dimension. We - * require at least 3 dimensions to validate settings, but you may set up to - * 32 dimensions. - * @param[in, out] settings The Zarr stream settings struct. - * @param[in] count The number of dimensions to reserve space for. - * @return ZarrStatus_Success on success, or an error code on failure. - */ - ZarrStatus ZarrStreamSettings_reserve_dimensions( - ZarrStreamSettings* settings, - size_t count); - - /** - * @brief Set properties for an acquisition dimension. - * @detail The order of the dimensions in the Zarr stream is the order in - * which they are set. The first dimension set is the slowest varying - * dimension, and the last dimension set is the fastest varying dimension. - * For example, if the dimensions are set in the order z, y, x, the fastest - * varying dimension is x, the next fastest varying dimension is y, and the - * slowest varying dimension is z. - * @param[in, out] settings The Zarr stream settings struct. - * @param[in] index The index of the dimension to set. Must be less than the - * number of dimensions reserved with ZarrStreamSettings_reserve_dimensions. - * @param[in] dimension The dimension's settings. - */ - ZarrStatus ZarrStreamSettings_set_dimension( - ZarrStreamSettings* settings, - size_t index, - const ZarrDimensionProperties* dimension); - - /** - * @brief Set the multiscale flag for the Zarr stream. - * @param[in, out] settings The Zarr stream settings struct. - * @param[in] multiscale A flag indicating whether to stream to multiple - * levels of detail. - * @return ZarrStatus_Success on success, or an error code on failure. - */ - ZarrStatus ZarrStreamSettings_set_multiscale(ZarrStreamSettings* settings, - uint8_t multiscale); - - /** - * @brief Set JSON-formatted custom metadata for the Zarr stream. - * @details This metadata will be written to acquire-zarr.json in the - * metadata directory of the Zarr store. This parameter is optional. - * @param settings[in, out] settings The Zarr stream settings struct. - * @param external_metadata JSON-formatted external metadata. - * @param bytes_of_external_metadata The length of @p custom_metadata in - * bytes, including the null terminator. - * @return ZarrStatus_Success on success, or an error code on failure. - */ - ZarrStatus ZarrStreamSettings_set_custom_metadata( - ZarrStreamSettings* settings, - const char* external_metadata, - size_t bytes_of_external_metadata); - - /** - * @brief Get the store path for the Zarr stream. - * @param settings The Zarr stream settings struct. - * @return The store path for the Zarr stream, or NULL on failure. - */ - const char* ZarrStreamSettings_get_store_path( - const ZarrStreamSettings* settings); - - /** - * @brief Get the S3 settings for the Zarr stream. - * @param settings The Zarr stream settings struct. - * @return The S3 settings for the Zarr stream, or an uninitialized struct on - * failure. - */ - ZarrS3Settings ZarrStreamSettings_get_s3_settings( - const ZarrStreamSettings* settings); - - /** - * @brief Get the compression settings for the Zarr stream. - * @param settings The Zarr stream settings struct. - * @return The compression settings for the Zarr stream, or an uninitialized - * struct on failure. - */ - ZarrCompressionSettings ZarrStreamSettings_get_compression( - const ZarrStreamSettings* settings); - - /** - * @brief Get the data type for the Zarr stream. - * @param settings The Zarr stream settings struct. - * @return The data type for the Zarr stream, or ZarrDataType_uint8 on failure. - */ - ZarrDataType ZarrStreamSettings_get_data_type( - const ZarrStreamSettings* settings); - - /** - * @brief Get the number of dimensions in the Zarr stream settings struct. - * @param settings The Zarr stream settings struct. - * @return The number of dimensions in the Zarr stream settings struct, or 0 on - * failure. - */ - size_t ZarrStreamSettings_get_dimension_count( - const ZarrStreamSettings* settings); - - /** - * @brief Get the properties for an acquisition dimension. - * @param settings The Zarr stream settings struct. - * @param index The index of the dimension to get. - * @return The properties for the @p index th dimension, or an uninitialized struct on - * failure. - */ - ZarrDimensionProperties ZarrStreamSettings_get_dimension( - const ZarrStreamSettings* settings, - size_t index); - - /** - * @brief Get the multiscale flag for the Zarr stream. - * @param settings The Zarr stream settings struct. - * @return The multiscale flag for the Zarr stream, or false on failure. - */ - bool ZarrStreamSettings_get_multiscale(const ZarrStreamSettings* settings); - - /** - * @brief Get the JSON-formatted custom metadata for the Zarr stream. - * @param settings The Zarr stream settings struct. - * @return The JSON-formatted custom metadata for the Zarr stream, or NULL on - * failure. - */ - const char* ZarrStreamSettings_get_custom_metadata( - const ZarrStreamSettings* settings); + const char* Zarr_get_status_message(ZarrStatusCode status); /** * @brief Create a Zarr stream. @@ -248,27 +75,11 @@ extern "C" * @param[out] bytes_out The number of bytes written to the stream. * @return ZarrStatus_Success on success, or an error code on failure. */ - ZarrStatus ZarrStream_append(ZarrStream* stream, + ZarrStatusCode ZarrStream_append(ZarrStream* stream, const void* data, size_t bytes_in, size_t* bytes_out); - /** - * @brief Get the version (i.e., 2 or 3) of the Zarr stream. - * @param stream The Zarr stream struct. - * @return The version of the Zarr stream. - */ - ZarrVersion ZarrStream_get_version(const ZarrStream* stream); - - /** - * @brief Get a copy of the settings for the Zarr stream. - * @param stream The Zarr stream struct. - * @return A copy of the settings for the Zarr stream. - */ - ZarrStreamSettings* ZarrStream_get_settings(const ZarrStream* stream); - #ifdef __cplusplus } #endif - -#endif // H_ACQUIRE_ZARR_V0 diff --git a/include/zarr.types.h b/include/zarr.types.h index 56a54a56..2dfd0dd1 100644 --- a/include/zarr.types.h +++ b/include/zarr.types.h @@ -23,7 +23,7 @@ extern "C" ZarrStatus_CompressionError, ZarrStatus_InvalidSettings, ZarrStatusCount, - } ZarrStatus; + } ZarrStatusCode; typedef enum { @@ -34,7 +34,7 @@ extern "C" typedef enum { - ZarrLogLevel_Debug, + ZarrLogLevel_Debug = 0, ZarrLogLevel_Info, ZarrLogLevel_Warning, ZarrLogLevel_Error, @@ -44,7 +44,7 @@ extern "C" typedef enum { - ZarrDataType_uint8, + ZarrDataType_uint8 = 0, ZarrDataType_uint16, ZarrDataType_uint32, ZarrDataType_uint64, @@ -87,13 +87,9 @@ extern "C" typedef struct { const char* endpoint; - size_t bytes_of_endpoint; const char* bucket_name; - size_t bytes_of_bucket_name; const char* access_key_id; - size_t bytes_of_access_key_id; const char* secret_access_key; - size_t bytes_of_secret_access_key; } ZarrS3Settings; /**