DM-55183: Fix problem in NDF of overwriting components - #50
Merged
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #50 +/- ##
==========================================
+ Coverage 74.20% 74.75% +0.54%
==========================================
Files 114 115 +1
Lines 13183 13436 +253
==========================================
+ Hits 9783 10044 +261
+ Misses 3400 3392 -8 ☔ View full report in Codecov by Harness. |
2 tasks
When two callers add an array, table, or pointer with the same logical name (e.g. each operand of a SumField calling add_array(name="data") from the same nested archive) hierarchical backends silently overwrote the first instance. Add a shared _register_name helper on the OutputArchive base class that returns (name, version) per logical name; each backend renders version > 1 in its own way. FITS already encoded a 1-based version as EXTVER; the helper replaces the previous _hdus_by_name Counter and produces an identical layout. NDF now appends _N to the leaf component before mapping to its HDF5 path so repeated names get sibling sub-NDFs. Includes a regression test that round-trips a SumField subtracted background through both backends.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-55183) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TallJimbo
approved these changes
Jun 10, 2026
Replace the lazy try/except initialization of _name_versions with an unconditional __init__ on the OutputArchive base class; derived archive classes now delegate to super().__init__(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_register_name now returns 0 for the first occurrence of a name; the
FITS backend adds one to produce the 1-based EXTVER and the NDF backend
renders later occurrences with a 1-based _{version + 1} suffix, so the
on-disk output is unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This reverts commit b680673cd0414ada36e74a9d27e6052dd446fcaf.
All the backends render the occurrence count 1-based on disk (the FITS
EXTVER keyword and the NDF _{version} suffix), so the 0-based return
value just forced every backend to add one back. Since the value is
a version rather than an index, 1-based is the more natural convention
after all. (DM-55183)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Since shrunk-name collisions can only occur within a single file written by a single process, a per-archive counter rendered as three uppercase hex digits replaces the blake2b digest. The new HdsNameShrinker class memoizes assignments so the same name always shrinks to the same result within a file, leaves a longer readable prefix (11 characters instead of 6), and makes collisions impossible rather than improbable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The two forms the normalization unified cannot collide: a direct add_array(name="data") and a FITS pointer target's nested add_array (which registers "/data") produce different EXTNAMEs (DATA vs /DATA), so sharing a dedupe key only bumped EXTVER spuriously. In the NDF backend names never reach _register_name with a leading slash because serialize_pointer roots its nested archive at the pointer's name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pin that repeated direct names get increasing EXTVERs and that a direct name and a pointer target's leading-slash name produce distinct EXTNAMEs with no spurious EXTVER bump. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
FITS was using EXTVERS,EXTNAME to allow HDUs with the same name to exist but NDF was overwriting. Rewrite to have FITS and NDF use the same tracking mechanism and then decide themselves what to do with the version value.
Also added a name shrinker to NDF writes to allow it to write cell coadds.
Checklist
doc/changes