Skip to content

Address TODOs, refactor repeated lines#29

Merged
mattcieslak merged 7 commits intomainfrom
refactor-1
Feb 25, 2026
Merged

Address TODOs, refactor repeated lines#29
mattcieslak merged 7 commits intomainfrom
refactor-1

Conversation

@mattcieslak
Copy link
Collaborator

There were a couple TODOs that suggested refactoring and a few lines of code that were repeated many times throughout the codebase. This PR adds some functions that reduce the repetition. Specifically:

  • Centralize type dispatch: Replace ~18 copy-pasted if/else dtype dispatch chains and ~51 scattered placement-new + reinterpret_cast sites with a single detail::remap() helper. Inside TrxFile<DT>, the template parameter is already known at compile time. The runtime dispatch was unnecessary.
  • Custom exception hierarchy: Replace all std::runtime_error / std::invalid_argument throws with library-specific exceptions (TrxIOError, TrxFormatError, TrxDTypeError, TrxArgumentError), all inheriting from TrxError : std::runtime_error. Existing catch(std::runtime_error) blocks continue to work.
  • Public accessors: Add data(), offsets(), lengths() on ArraySequence and matrix() on MMappedMatrix.
  • Make load_from_zip/load_from_directory public on TrxFile.
  • Remove the #define private public hack from tests.
  • RAII wrappers for libzip: detail::ZipArchive and detail::ZipFile eliminate manual zip_close/zip_fclose cleanup in error paths across all zip-handling code.
  • Rewrite deepcopy(): Replace the write-to-disk-then-reload round-trip with direct _initialize_empty_trx + Eigen block copies.
  • TrxStream API cleanup: set_dimensions, set_voxel_to_rasmm, set_metadata_mode, set_metadata_buffer_max_bytes now return TrxStream& for chaining.
  • Replace .compare() with == across ~23 string comparisons.
  • Resolve ~10 TODO comments (fixed, documented as known limitations, or removed after the underlying issue was addressed).

@codecov
Copy link

codecov bot commented Feb 22, 2026

Codecov Report

❌ Patch coverage is 63.55476% with 203 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.86%. Comparing base (39a542f) to head (7caf6b3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
include/trx/trx.tpp 64.12% 113 Missing ⚠️
src/trx.cpp 41.37% 85 Missing ⚠️
include/trx/detail/zip_raii.h 78.26% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #29      +/-   ##
==========================================
+ Coverage   85.25%   86.86%   +1.61%     
==========================================
  Files          13       14       +1     
  Lines        6076     5809     -267     
  Branches      946      881      -65     
==========================================
- Hits         5180     5046     -134     
+ Misses        896      763     -133     
Flag Coverage Δ
linux 86.78% <62.93%> (+1.68%) ⬆️
macos 86.78% <63.35%> (+1.61%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

@mattcieslak mattcieslak merged commit 1a8c580 into main Feb 25, 2026
8 checks passed
@mattcieslak mattcieslak deleted the refactor-1 branch February 25, 2026 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant