Add fixed shape tensor #18
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for the Arrow canonical extension type arrow.fixed_shape_tensor, which enables storing fixed-shape tensors in Arrow format. The implementation follows the Apache Arrow specification for fixed shape tensor arrays.
Key Changes
- Introduces
fixed_shape_tensor_arrayclass wrappingfixed_sized_list_arraywith tensor-specific metadata - Implements JSON serialization/deserialization for tensor metadata (shape, dimension names, permutation)
- Replaces nlohmann_json dependency with simdjson for improved JSON parsing performance
- Adds comprehensive test coverage with 752 lines of tests
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| include/sparrow_extensions/fixed_shape_tensor.hpp | Defines the fixed_shape_tensor_array class and metadata structures with comprehensive documentation |
| src/fixed_shape_tensor.cpp | Implements metadata validation, JSON serialization, and array construction logic |
| tests/test_fixed_shape_tensor.cpp | Provides extensive test coverage for metadata operations, array construction, and spec compliance |
| cmake/external_dependencies.cmake | Adds simdjson dependency and updates doctest version |
| environment-dev.yml | Replaces nlohmann_json with simdjson in conda dependencies |
| CMakeLists.txt | Registers new source files and headers in build system |
| include/sparrow_extensions.hpp | Includes the new fixed_shape_tensor header in main library header |
| tests/CMakeLists.txt | Adds test file to test suite |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fixed_shape_tensor_array& operator=(const fixed_shape_tensor_array&) = default; | ||
| fixed_shape_tensor_array(fixed_shape_tensor_array&&) noexcept = default; | ||
| fixed_shape_tensor_array& operator=(fixed_shape_tensor_array&&) noexcept = default; | ||
| ~fixed_shape_tensor_array() = default; |
There was a problem hiding this comment.
No need to declare them if they are all defaulted.
| public: | ||
|
|
||
| using size_type = std::size_t; | ||
| using metadata_type = fixed_shape_tensor_extension::metadata; |
There was a problem hiding this comment.
It might be worth adding inner typedefs like we have for regular arrays (value_type, const_reference, reference, iterators etc).
| * @pre i < size() | ||
| */ | ||
| [[nodiscard]] auto operator[](size_type i) const | ||
| -> decltype(std::declval<const sparrow::fixed_sized_list_array&>()[i]); |
There was a problem hiding this comment.
This method could return const_reference if defined as an inner type as suggested above.
2 participants
No description provided.