Reflection table improvements#49
Merged
Merged
Conversation
Expose a public size() method that returns the number of rows (reflections) in the table, delegating to the existing private get_row_count() helper. Previously callers had to fetch an arbitrary column and inspect its extent(0) to determine the row count. Closes #20
Open the output file with H5Fcreate/H5F_ACC_TRUNC so each write produces a clean file. Previously an existing file was opened with H5F_ACC_RDWR, which only overwrote same-named datasets in place: any datasets from a prior write that were absent from the current table were left behind as stale data (it did not append rows, as first suspected). Truncating guarantees the file contains only the current table's columns. Closes #19
Previously add_column threw if the column name already existed, with no way to replace a column's values. Add a strict update_column<T> (shape, 1D and 2D overloads) that requires the column to exist, validates type and row-count consistency, and replaces it in place (preserving column order). Factor the type-support check into a shared ensure_supported_type helper used by both add_column and update_column. Closes #18
jbeilstenedmands
approved these changes
Jun 18, 2026
jbeilstenedmands
left a comment
Contributor
There was a problem hiding this comment.
Thanks for addressing these issues and adding clear tests for the expected functionality. I have also tested the new features work as expected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR works through the open items collected in #15, which came out
of using the DX2
ReflectionTablein the fast-feedback-service indexingalgorithm. Each was a small gap in the table's public API or a
surprising write-time behaviour; they are addressed as three
self-contained commits.
Changes:
Add a method to return the reflection table size #20
Add
ReflectionTable::size(). Exposes the number of rows(reflections) as a public method (
include/dx2/reflection.hpp,dx2/reflection.cxx), delegating to the existing privateget_row_count()helper. Previously callers had to fetch an arbitrarycolumn and read its
extent(0)just to learn the row count.Reflection table writing appears to update an existing file #19
Truncate the output file on
write().ReflectionTable::writenow opens the target withH5Fcreate/H5F_ACC_TRUNC(dx2/reflection.cxx). It previouslyopened an existing file with
H5F_ACC_RDWRand only overwrotesame-named datasets in place, so datasets from a prior write that were
absent from the current table were left behind as stale data (it did
not append rows, as first suspected). Truncating guarantees the file
contains only the current table's columns.
Allow reassignment of reflection column data? #18
Add
ReflectionTable::update_column. Adds a strictupdate_column<T>with shape-based, 1D and 2D overloads(
include/dx2/reflection.hpp) mirroringadd_column. It requires thenamed column to already exist (throwing otherwise), validates type
support and row-count consistency with the rest of the table, and
replaces the column in place to preserve column order.
add_columnkeeps its duplicate-name safety net, so the previous "check if
present, else add" workaround is no longer needed. The shared
type-support check is factored into a private
ensure_supported_type<T>()helper reused by both methods.Each change is covered by new gtest cases in
tests/test_reflection_table.cxx(size of empty/populated/loaded tables, overwrite-not-update on
re-write, and update_column replace / missing / row-count-mismatch /
order-preservation).
Closes #15.
Closes #18.
Closes #19.
Closes #20.