Skip to content

Add ros3 S3 read support#307

Open
oruebel wants to merge 59 commits into
mainfrom
add_ros3
Open

Add ros3 S3 read support#307
oruebel wants to merge 59 commits into
mainfrom
add_ros3

Conversation

@oruebel

@oruebel oruebel commented Jul 13, 2026

Copy link
Copy Markdown
Contributor
  • Add HDF5IO.open_s3 method
  • Fixed bug in HDF5IO.canModifyObject to work with ROS3
  • Add brief example and tutorial

Changes from #308

  • Added simple demo for benchmarking ROS3 performance
  • Updated CI for demo runs to use Ubuntu and add the new ROS3 demo
  • Added new BaseIO::findObject and RegisteredType::findOwnedObject methods to simplify searching for objects by name
  • Added override for HDF5IO::findObject to improve search performance
  • Fixed install-config.cmake to propagate HDF5 include directories to downstream consumers of the aqnwb::aqnwb target, allowing them to locate HDF5 headers regardless of where HDF5 is installed on the system.
  • Merged changes from Add remfile VFD for reading remote files over HTTP(S) #309 which adds remfile support

Changes from #309

  • Added HDF5IO::openRemote() method to read remote NWB files over HTTP(S) using the remfile-cpp virtual file driver (a C++ port of the Python remfile package), imported as an optional CMake dependency (AQNWB_USE_REMFILE, ON by default on non-Windows platforms; requires libcurl). Unlike ROS3, remfile does not require HDF5 to be built with ROS3 support and works with any HTTP(S) server that supports byte-range requests. The ros3_benchmark demo accepts an optional driver argument (ros3 or remfile) to compare the two.

Changes after merging the different PR's

  • Updated remfile build in CMakeLists to use the main branch instead of the pinned version 0.1.2 which failed on Windows
  • Renamed the demo/ros3_benchmark to demo/remote_read_benchmark since it now covers both remfile and ros3

NOTE: The ROS3 unit test and example are currently only run in CI in the Ubuntu tests. MacOS and Windows CI do not install HDF5 with ROS3 support enabled by default. However, testing on one platform should be sufficient for this feature and this also confirms that the optional build with ROS3 works. So no changes to the CI needed.

NOTE: the failing CI for [Test generating classes from schema files / run-scripts (pull_request)] is due to changes in the NWB 2.10 schema and has been addressed separately in this PR #300 .

@codecov-commenter

codecov-commenter commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.73469% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.56%. Comparing base (6571d66) to head (4723565).

Files with missing lines Patch % Lines
src/io/hdf5/HDF5IO.cpp 86.36% 9 Missing ⚠️
src/nwb/RegisteredType.cpp 57.14% 3 Missing ⚠️
src/io/BaseIO.cpp 96.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #307      +/-   ##
==========================================
+ Coverage   84.52%   84.56%   +0.04%     
==========================================
  Files          57       57              
  Lines        2591     2682      +91     
  Branches      332      347      +15     
==========================================
+ Hits         2190     2268      +78     
- Misses        401      414      +13     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@oruebel oruebel added category: enhancement proposed enhancements or new features priority: low alternative solution already working and/or relevant to only specific user(s) labels Jul 13, 2026
@oruebel oruebel self-assigned this Jul 13, 2026
@oruebel oruebel added this to the 0.4.0 milestone Jul 13, 2026
@oruebel
oruebel marked this pull request as ready for review July 13, 2026 06:37
@oruebel
oruebel requested a review from rly July 13, 2026 06:37
oruebel and others added 17 commits July 12, 2026 23:55
Imports the remfile-cpp package (https://github.com/bendichter/remfile-cpp,
a C++ port of the Python remfile package) as an optional CMake dependency
and adds HDF5IO::openRemote() to open remote files with it. Unlike ROS3,
the remfile VFD does not require HDF5 to be built with ROS3 support and
works with any HTTP(S) server that supports byte-range requests.

The ros3_benchmark demo accepts an optional driver argument
(ros3 | remfile) so the two drivers can be benchmarked head-to-head,
and CI runs the demo with both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@oruebel

oruebel commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Completed review

Comment thread CMakeLists.txt Outdated
Comment thread CMakeLists.txt Outdated
FetchContent_Declare(
remfile
GIT_REPOSITORY https://github.com/catalystneuro/remfile-cpp.git
# GIT_TAG v0.1.2

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to fix the GIT_TAG once remfile-cpp has been released with the fix. The GIT_TAG has been removed for now in order to confirm with the dev version that this is working.

Suggested change
# GIT_TAG v0.1.2
GIT_TAG v0.1.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: enhancement proposed enhancements or new features priority: low alternative solution already working and/or relevant to only specific user(s)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants